feat(quest-details): add support for play-streak progress sync

[eliobricenov]

No description provided.

[eliobricenov]

I'm purposely omitting this because I don't think we should return the sync handler here. We'd have to include all the props from UseSyncPlayStreakProps

[eliobricenov]

This is the implementation on the client:

const { syncPlayStreak } = useSyncPlayStreak({
    refreshPlayStreak: questPlayStreakResult.invalidateQuery
})

<QuestDetails
  ...
    playStreak: {
    ...getPlaystreakArgsFromQuestData(questMeta, questPlayStreakData),
        onSync: async () => syncPlayStreak(selectedQuestId)
    }
/>

[BrettCleary]

Why not add useSyncPlayStreak directly to the quest details wrapper component above? This way the re-renders are contained to just the wrapper component.

This fits the existing pattern like for:

  useSyncPlaySession(
    projectId,
    questPlayStreakResult.invalidateQuery,
    syncPlaySession
  )

where we pass in the syncPlaySession function as a prop. (which is window.api on client and a fetch elsewhere).

so you would just take in

syncPlayStreakWithExternalSource: (params: {
    quest_id: number
    signature: string
  }) => Promise<unknown>

as a quest details wrapper prop and pass it to useSyncPlayStreak

[BrettCleary]

you also wouldn't need to pass in all the props in this case. just the mutation fxn call returned from useSyncPlayStreak

[eliobricenov]

My main target was to prevent having to include another required prop to the QuestRewardWrapper props because this is technically a prop that won't be used unless the quest type is play streak.

The play streak prop is optional and only used for play streak quests, so that was my main reason for choosing this place.

On second thought, it's fine to add it. I don't love its API, but it's fine, this is a pretty niche and complex implementation that won't be used elsewhere.

[BrettCleary]

I think we can make a few changes to match the existing patterns a bit better

[BrettCleary]

Why not add useSyncPlayStreak directly to the quest details wrapper component above? This way the re-renders are contained to just the wrapper component.

This fits the existing pattern like for:

  useSyncPlaySession(
    projectId,
    questPlayStreakResult.invalidateQuery,
    syncPlaySession
  )

where we pass in the syncPlaySession function as a prop. (which is window.api on client and a fetch elsewhere).

so you would just take in

syncPlayStreakWithExternalSource: (params: {
    quest_id: number
    signature: string
  }) => Promise<unknown>

as a quest details wrapper prop and pass it to useSyncPlayStreak

[BrettCleary]

you also wouldn't need to pass in all the props in this case. just the mutation fxn call returned from useSyncPlayStreak

[eliobricenov]

I like that the logic of when to execute an external sync is here. The drawback is the need of questsWithExternalPlayStreakSync but I think that's ok

[BrettCleary]

one small comment

[BrettCleary]

you need to also call resetSessionStartedTime if you're going to call syncPlaySession here

[BrettCleary]

and you may need a delay between the syncPlaySession call and the invalidate query like in the hook:

      await syncPlaySession(projectId, 'hyperplay')
      // allow for some time before read
      await wait(5000)
      await invalidateQuery()
      resetSessionStartedTime()

[eliobricenov]

done, I'll try not to add the wait to check if we truly need it. I tested without it, and it's working fine