Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[User model] [Fix] macOS Safari prompting and subscribing #1062

Merged
merged 4 commits into from
Jul 28, 2023
Merged

[User model] [Fix] macOS Safari prompting and subscribing #1062

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b724a62
remove no longer needed first page view check
jkasten2 Jul 26, 2023
04c97ae
remove unnecessary optedOut check
jkasten2 Jul 26, 2023
03d5989
fix safari legacy push token detection
jkasten2 Jul 26, 2023
6ede032
updated Safari 16 comments
jkasten2 Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 2 additions & 24 deletions src/shared/helpers/SubscriptionHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,18 @@ import Log from '../libraries/Log';
import { ContextSWInterface } from '../models/ContextSW';
import SdkEnvironment from '../managers/SdkEnvironment';
import { EnvironmentInfo } from '../../page/models/EnvironmentInfo';
import { SessionOrigin } from '../models/Session';
import { Browser } from '../models/Browser';
import { PushDeviceRecord } from '../models/PushDeviceRecord';
import { PermissionUtils } from '../utils/PermissionUtils';
import MainHelper from './MainHelper';

export default class SubscriptionHelper {
public static async registerForPush(): Promise<Subscription | null> {
const isPushEnabled = await OneSignal.context.subscriptionManager.isPushNotificationsEnabled();
return await SubscriptionHelper.internalRegisterForPush(isPushEnabled);
return await SubscriptionHelper.internalRegisterForPush();
}

public static async internalRegisterForPush(isPushEnabled: boolean): Promise<Subscription | null> {
public static async internalRegisterForPush(): Promise<Subscription | null> {
const context: ContextSWInterface = OneSignal.context;
let subscription: Subscription | null = null;

/*
Within the same page navigation (the same session), do not register for
push if the user is already subscribed, otherwise the user will have its
session count incremented on each page refresh. However, if the user is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you confirm the session count is not incrementing even after the refresh? What is the impact on other browsers? From first glance it doesn't appear this code was Safari-specific.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the "Tests" section in this PR to provide more details on this.

not subscribed, subscribe.
*/
if (isPushEnabled && !context.pageViewManager.isFirstPageView()) {
Log.debug('Not registering for push because the user is subscribed and this is not the first page view.');
Log.debug("But we want to rekindle their session.");
const deviceId = await MainHelper.getDeviceId();
if (deviceId) {
const deviceRecord: PushDeviceRecord = await MainHelper.createDeviceRecord(OneSignal.config.appId, true);
await OneSignal.context.sessionManager.upsertSession(deviceId, deviceRecord, SessionOrigin.PageRefresh);
} else {
Log.error("Should have been impossible to have push as enabled but no device id.");
}
return null;
}

switch (SdkEnvironment.getWindowEnv()) {
case WindowEnvironmentKind.Host:
Expand Down