at_client_sdk
at_client_sdk copied to clipboard
feat: uptake notify fetch changes
- What I did
- Add 'fetch' method to NotificationService class and fetches the AtNotification from the cloud secondary using the notification id
- How I did it
-
Files Modified:
-
- at_notification.dart - When a notification is expired, the notification is removed from the keystore and hence details cannot be fetched to populate the fields in AtNotification class (except the id). So, removed the late modifier and initialized the fields to empty string and epochMillis to '0 (Zero)' to avoid LateInitialization error when accessing the AtNotification.
-
- Since
AtNotification()
has arguments that are available when notification is expired, added a named constructorAtNotification.empty
and populate the fields.
- Since
-
- Added unit tests for the scenario's - when notification is available in keystore - when notification is not available in keystore
-
- Removed the skip tag on end2end test and refactored to fetch the notification using
notify fetch
and added assertions
- Removed the skip tag on end2end test and refactored to fetch the notification using
- How to verify it
- Description for the changelog
A few comments /. questions /. changes requested.
Couple of other things:
- please add to the functional test suite in at_functional_test/test/atclient_notify_test.dart
- please add functional and end to end tests for fetches on expired notifications
- please add functional and end to end tests for fetches on non-existent notifications
- please add unit tests for every exception scenario for fetch
- Added tests in at_functional_tests
- Added test in e2e for non-existent notifications
- Added unit tests for exception scenario
- At the moment, we do not have a way to set the expiry on the notification. We have a expose-notification-params PR which allows to add expiry to notifications. Can we add fetch notification on expiry in that PR?
- At the moment, we do not have a way to set the expiry on the notification. We have a expose-notification-params PR which allows to add expiry to notifications. Can we add fetch notification on expiry in that PR?
I don't understand ... we can set ttln on a notification. When time-to-live is set, then when the time-to-live duration has passed, the notification has expired, no?
- At the moment, we do not have a way to set the expiry on the notification. We have a expose-notification-params PR which allows to add expiry to notifications. Can we add fetch notification on expiry in that PR?
I don't understand ... we can set ttln on a notification. When time-to-live is set, then when the time-to-live duration has passed, the notification has expired, no?
At the moment, in at_client SDK, we do not have a way to set ttln on the NotificationParams. We have an open PR which exposes notificationExpiry.
This LGTM. @VJag also had comments & questions, please don't merge until @VJag has also re-reviewed & approved
Sure Gary