Added limit and offset to HereNow API. #376
Conversation
…make behaviour consistent with multi-channel calls.
Added deprecation warning for old APNS. APNS2 should be used instead. In case FCM is chosen as PushType type REST query param gets fcm value instead of gcm.
marcin-cebo
requested review from
jakub-grzesiowski,
jguz-pubnub,
kleewho and
parfeon
as code owners
| } | ||
|
|
||
| @Test | ||
| public void testGlobalHereNowWithOffset() throws PubNubException { |
There was a problem hiding this comment.
Are you sure global HereNow supports limit and offset parameters?
There was a problem hiding this comment.
Yes, somehow it does.
| return HereNowImpl( | ||
| jsPubNub, | ||
| createJsObject { | ||
| // todo handle limit and offset |
There was a problem hiding this comment.
Is additional work required due to this comment?
There was a problem hiding this comment.
Yes, additional work is required for JS and Swift part of KMP. But will be handled in separate PR
| "Channel and/or ChannelGroup contains empty string which is not allowed.", | ||
| ), | ||
|
|
||
| HERE_NOW_OFFSET_OUT_OF_RANGE( |
There was a problem hiding this comment.
It’s better to use UInt for limit and offset to ensure they’re never negative, which eliminates the need for extra validations. Otherwise, keep them as Int and just document that the values must be greater than zero. No need for defensive checks.
There was a problem hiding this comment.
Good point. Uint could solve validation problem but we don't use Uint because code is also used by Java users and in Java there is no Uint and it would not comfortable to convert int to Uint. Documentation contains info about them being greater than zero.
No need for defensive checks.
What problem do you see with those checks?
There was a problem hiding this comment.
After discussion on Daily I will remove some validations.
|
Change the PR title that reflect the scope of changes |
…make behaviour consistent with multi-channel calls.
Added deprecation warning for old APNS. APNS2 should be used instead. In case FCM is chosen as PushType type REST query param gets fcm value instead of gcm.
|
@pubnub-release-bot release kotlin as v11.0.0 |
|
🚀 Release successfully completed 🚀 |
3 participants
feat: Added limit and offset parameters for hereNow
Added limit and offset parameters for hereNow. Number of returned users per channel by default is limited to 1000. Breaking change
fix: Single-channel hereNow with includeUUIDs=false now returns channel data
Single-channel hereNow with includeUUIDs=false now returns channel data in the channels map for consistency with multi-channel behaviour. Previously, the channels map was empty in this scenario, forcing reliance on totalOccupancy field only. Breaking change
fix: Removed possibility to use deprecated MPNS -Microsoft Push Notification Service. Breaking change.
fix: Added deprecation warning for GCP and old APNS PushType.
fix: In case FCM is chosen as PushType type REST query param gets fcm value instead of gcm.