Do not return daily challenge rooms in GET /rooms endpoint if API version too low
#11240
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ersion too low Currently old lazer clients die on these due to deserialisation woes (it can't figure out what enum value to deserialise `daily_challenge` to as it doesn't exist yet), which makes deployment of the feature a bit tricky logistically. Since we can't fix old clients (because they're already released, and we would have to push an update either way), hide daily challenge rooms behind an `x-api-version` header check. Note that challenge rooms can still be accessed directly by ID regardless of API version, but that's fine for this case, since the client won't be doing it.
bdach
commented
May 29, 2024
Comment on lines
+52
to
+53
| // can be removed 20241129 | ||
| if ($apiVersion < 20240529) { |
There was a problem hiding this comment.
Not sure if you do this sort of thing around these parts, but I don't foresee this living too long, so I did the standard that we do in client of only keeping this around for ~6 months at which point it can be nuked.
There was a problem hiding this comment.
maybe add note that it's temporary in the docs?
There was a problem hiding this comment.
208600c should do it?
bdach
added a commit
to bdach/osu
that referenced
this pull request
May 29, 2024
(Or local date, in the case of non-deployed builds). Came up when I was looking at ppy/osu-web#11240 and found that we were still hardcoding this. Thankfully, this *should not* cause issues, since there don't seem to be any (documented or undocumented) API response version checks for versions newer than 20220705 in osu-web master. For clarity and possible debugging needs, the API response version is also logged.
nanaya
approved these changes
May 29, 2024
TextAdventurer12
pushed a commit
to TextAdventurer12/osu
that referenced
this pull request
Jul 6, 2024
(Or local date, in the case of non-deployed builds). Came up when I was looking at ppy/osu-web#11240 and found that we were still hardcoding this. Thankfully, this *should not* cause issues, since there don't seem to be any (documented or undocumented) API response version checks for versions newer than 20220705 in osu-web master. For clarity and possible debugging needs, the API response version is also logged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RFC.
Currently old lazer clients die on these due to deserialisation woes (it can't figure out what enum value to deserialise
daily_challengeto as it doesn't exist yet), which makes deployment of the feature a bit tricky logistically.Since we can't fix old clients (because they're already released, and we would have to push an update either way), hide daily challenge rooms behind an
x-api-versionheader check. Note that challenge rooms can still be accessed directly by ID regardless of API version, but that's fine for this case, since the client won't be doing it.