feat(in-app-messaging): Add web support for session cap #9308

cshfang · 2021-12-03T20:20:47Z

Description of changes

Previously, session tracking for the purposes of Pinpoint in-app messaging campaign session caps only worked with React Native. This PR implements the session tracker for web to extend that feature to the web.

Issue #, if available

N/A

Description of how you validated changes

Created a web app and tested session cap mechanism to see that:

Session caps were enforced
Session caps were reset when browser visibility went from inactive to active

Tested with Chrome, Firefox, Safari and Edge

Checklist

PR description included
yarn test passes
Tests are changed or added
Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter · 2021-12-03T20:35:46Z

Codecov Report

Merging #9308 (10a5109) into in-app-messaging/staging (8c1088c) will decrease coverage by 0.05%.
The diff coverage is 50.00%.

@@                     Coverage Diff                      @@
##           in-app-messaging/staging    #9308      +/-   ##
============================================================
- Coverage                     74.37%   74.32%   -0.06%     
============================================================
  Files                           312      312              
  Lines                         20148    20177      +29     
  Branches                       4385     4394       +9     
============================================================
+ Hits                          14986    14997      +11     
- Misses                         5023     5041      +18     
  Partials                        139      139

Impacted Files	Coverage Δ
...rc/InAppMessaging/SessionTracker/SessionTracker.ts	`54.28% <50.00%> (-45.72%)`	⬇️
...kages/amazon-cognito-identity-js/src/BigInteger.js	`89.31% <0.00%> (-0.41%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c1088c...10a5109. Read the comment docs.

Samaritan1011001

Thank you! Looks good. Just two questions.

Samaritan1011001 · 2021-12-03T20:53:52Z

packages/notifications/src/InAppMessaging/SessionTracker/SessionTracker.ts

+if (typeof document.hidden !== 'undefined') {
+	hidden = 'hidden';
+	visibilityChange = 'visibilitychange';
+} else if (typeof document['msHidden'] !== 'undefined') {
+	hidden = 'msHidden';
+	visibilityChange = 'msvisibilitychange';
+} else if (typeof document['webkitHidden'] !== 'undefined') {
+	hidden = 'webkitHidden';
+	visibilityChange = 'webkitvisibilitychange';
+}


Small nit: document.hidden and document['msHidden'] . The way the value is accessed is different.

And a tiny question, this needs to run only once?

It should only need to run once as it's setting up vendor differences for browsers only.

document seems to be typed so that the vendor prefixed versions are not readily available, hence the different accessors as dot notation would error.

Samaritan1011001 · 2021-12-03T20:56:20Z

packages/notifications/src/InAppMessaging/SessionTracker/SessionTracker.ts

+		return this.getSessionState();
+	};
+
+	private getSessionState = (): SessionState => {


Observation: return type is SessionState but this function returns a string.

Ah the typing for SessionState is as follows:

export type SessionState = 'started' | 'ended';

Ah! right 👍

elorzafe

I have some concerns if document is undefined, can it be a Native implementation on one for the browser?

elorzafe · 2021-12-03T23:18:34Z

packages/notifications/src/InAppMessaging/SessionTracker/SessionTracker.ts

+let hidden: string;
+let visibilityChange: string;
+
+if (typeof document.hidden !== 'undefined') {


Can document be undefined?

As this code should not run on React Native, there should be no reason it's undefined. However, there's no harm in adding a check in case it does run in some unsupported environment for some reason.

elorzafe

LGTM 🌮

chintannp · 2021-12-06T19:08:27Z

LGTM 🚀

…saging/main (#9365) * chore(in-app-message): In app messaging/add unit tests (#9253) * chore: Add unit tests * Add licensing to top of analytics test * Fixed typo * Addressed PR feedback * Remove quiet time test Co-authored-by: Chris Fang <chrfang@amazon.com> * fix(@aws-amplify/datastore): consecutive saves with timestamps (#9298) * chore: preparing release * chore(release): Publish [ci skip] - @aws-amplify/ui-angular@1.0.36 - @aws-amplify/ui-components@1.9.7 - @aws-amplify/ui-react@1.2.27 - @aws-amplify/ui-storybook@2.0.27 - @aws-amplify/ui-vue@1.1.21 - @aws-amplify/ui@2.0.4 - @aws-amplify/analytics@5.1.7 - @aws-amplify/api-graphql@2.2.16 - @aws-amplify/api-rest@2.0.27 - @aws-amplify/api@4.0.27 - @aws-amplify/auth@4.3.17 - aws-amplify-angular@6.0.27 - aws-amplify-react@5.1.10 - aws-amplify-vue@2.1.6 - aws-amplify@4.3.9 - @aws-amplify/cache@4.0.29 - @aws-amplify/core@4.3.9 - @aws-amplify/datastore-storage-adapter@1.2.1 - @aws-amplify/datastore@3.7.1 - @aws-amplify/geo@1.1.9 - @aws-amplify/interactions@4.0.27 - @aws-amplify/predictions@4.0.27 - @aws-amplify/pubsub@4.2.3 - @aws-amplify/pushnotification@4.3.6 - @aws-amplify/storage@4.4.10 - @aws-amplify/xr@3.0.27 * chore(release): update version.ts [ci skip] * fix(@aws-amplify/amplify-ui): change private property to false in package.json (#9303) Resolving an issue with installing the library * chore: preparing release * fix(@aws-amplify/amplify-ui): remove private prop from package.json (#9304) * fix(@aws-amplify/amplify-ui): change private property to false in package.json * fix(@aws-amplify/amplify-ui): remove private property from package.json Co-authored-by: Francisco Rodriguez <frodriguez.cs@gmail.com> * fix(@aws-amplify/amplify-ui): remove private prop from package.json (#9304) * fix(@aws-amplify/amplify-ui): change private property to false in package.json * fix(@aws-amplify/amplify-ui): remove private property from package.json Co-authored-by: Francisco Rodriguez <frodriguez.cs@gmail.com> * chore(release): Publish [ci skip] - @aws-amplify/ui-angular@1.0.37 - @aws-amplify/ui-components@1.9.8 - @aws-amplify/ui-react@1.2.28 - @aws-amplify/ui-storybook@2.0.28 - @aws-amplify/ui-vue@1.1.22 - @aws-amplify/ui@2.0.5 - @aws-amplify/analytics@5.1.8 - @aws-amplify/api-graphql@2.2.17 - @aws-amplify/api-rest@2.0.28 - @aws-amplify/api@4.0.28 - @aws-amplify/auth@4.3.18 - aws-amplify-angular@6.0.28 - aws-amplify-react@5.1.11 - aws-amplify@4.3.10 - @aws-amplify/cache@4.0.30 - @aws-amplify/core@4.3.10 - @aws-amplify/datastore-storage-adapter@1.2.2 - @aws-amplify/datastore@3.7.2 - @aws-amplify/geo@1.1.10 - @aws-amplify/interactions@4.0.28 - @aws-amplify/predictions@4.0.28 - @aws-amplify/pubsub@4.2.4 - @aws-amplify/pushnotification@4.3.7 - @aws-amplify/storage@4.4.11 - @aws-amplify/xr@3.0.28 * chore(release): update version.ts [ci skip] * fix(@aws-amplify/datastore): fixes observeQuery in local-only mode (#9300) * feat(in-app-messaging): Add web support for session cap (#9308) * feat(in-app-messaging): Add web support for session cap * Add undefined check for document Co-authored-by: Chris Fang <chrfang@amazon.com> * fix(@aws-amplify/pushnotification): make eligible variables final (#9301) Co-authored-by: Manoj NB <manojnb@amazon.com> Co-authored-by: Caleb Pollman <cpollman1@gmail.com> * fix(@aws-amplify/aws-amplify-react-native): fix dev build for Windows (#9341) * fix(@aws-amplify/api-graphql): Fix webpack build (#9358) * chore: preparing release * chore(release): Publish [ci skip] - @aws-amplify/ui-angular@1.0.38 - @aws-amplify/ui-components@1.9.9 - @aws-amplify/ui-react@1.2.29 - @aws-amplify/ui-storybook@2.0.29 - @aws-amplify/ui-vue@1.1.23 - @aws-amplify/analytics@5.1.9 - @aws-amplify/api-graphql@2.2.18 - @aws-amplify/api-rest@2.0.29 - @aws-amplify/api@4.0.29 - @aws-amplify/auth@4.3.19 - aws-amplify-angular@6.0.29 - aws-amplify-react-native@6.0.2 - aws-amplify-react@5.1.12 - aws-amplify@4.3.11 - @aws-amplify/cache@4.0.31 - @aws-amplify/core@4.3.11 - @aws-amplify/datastore-storage-adapter@1.2.3 - @aws-amplify/datastore@3.7.3 - @aws-amplify/geo@1.1.11 - @aws-amplify/interactions@4.0.29 - @aws-amplify/predictions@4.0.29 - @aws-amplify/pubsub@4.2.5 - @aws-amplify/pushnotification@4.3.8 - @aws-amplify/storage@4.4.12 - @aws-amplify/xr@3.0.29 * chore(release): update version.ts [ci skip] * chore(in-app-messaging): upgrade internal dependency versions and notifications version (#9364) Co-authored-by: Chris F <5827964+cshfang@users.noreply.github.com> Co-authored-by: Chris Fang <chrfang@amazon.com> Co-authored-by: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Co-authored-by: aws-amplify-bot <aws@amazon.com> Co-authored-by: Nick Arocho <16296496+nickarocho@users.noreply.github.com> Co-authored-by: Nick Arocho <nicaroch@amazon.com> Co-authored-by: Francisco Rodriguez <frodriguez.cs@gmail.com> Co-authored-by: ManojNB <manojnb95@gmail.com> Co-authored-by: Manoj NB <manojnb@amazon.com> Co-authored-by: David McAfee <mcafd@amazon.com> Co-authored-by: Katie Goines <katiegoi@amazon.com>

github-actions · 2022-12-07T02:03:19Z

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

feat(in-app-messaging): Add web support for session cap

6d21140

cshfang requested review from chintannp, elorzafe and Samaritan1011001 December 3, 2021 20:39

Samaritan1011001 approved these changes Dec 3, 2021

View reviewed changes

elorzafe reviewed Dec 3, 2021

View reviewed changes

Add undefined check for document

10a5109

cshfang requested a review from elorzafe December 3, 2021 23:48

elorzafe approved these changes Dec 6, 2021

View reviewed changes

cshfang merged commit 551baa1 into aws-amplify:in-app-messaging/staging Dec 6, 2021

cshfang deleted the in-app-messaging/web-session-tracker branch December 6, 2021 19:32

github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(in-app-messaging): Add web support for session cap #9308

feat(in-app-messaging): Add web support for session cap #9308

cshfang commented Dec 3, 2021

codecov-commenter commented Dec 3, 2021 •

edited

Loading

Samaritan1011001 left a comment

Samaritan1011001 Dec 3, 2021

cshfang Dec 3, 2021

Samaritan1011001 Dec 3, 2021

cshfang Dec 3, 2021

Samaritan1011001 Dec 3, 2021

elorzafe left a comment

elorzafe Dec 3, 2021

cshfang Dec 3, 2021

elorzafe left a comment

chintannp commented Dec 6, 2021

github-actions bot commented Dec 7, 2022

feat(in-app-messaging): Add web support for session cap #9308

feat(in-app-messaging): Add web support for session cap #9308

Conversation

cshfang commented Dec 3, 2021

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

codecov-commenter commented Dec 3, 2021 • edited Loading

Codecov Report

Samaritan1011001 left a comment

Choose a reason for hiding this comment

Samaritan1011001 Dec 3, 2021

Choose a reason for hiding this comment

cshfang Dec 3, 2021

Choose a reason for hiding this comment

Samaritan1011001 Dec 3, 2021

Choose a reason for hiding this comment

cshfang Dec 3, 2021

Choose a reason for hiding this comment

Samaritan1011001 Dec 3, 2021

Choose a reason for hiding this comment

elorzafe left a comment

Choose a reason for hiding this comment

elorzafe Dec 3, 2021

Choose a reason for hiding this comment

cshfang Dec 3, 2021

Choose a reason for hiding this comment

elorzafe left a comment

Choose a reason for hiding this comment

chintannp commented Dec 6, 2021

github-actions bot commented Dec 7, 2022

codecov-commenter commented Dec 3, 2021 •

edited

Loading