Skip to content

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Jun 12, 2018


This change is Reviewable

* Fixes "Cannot read property 'force' of undefined" introduced in 150201
* resolves #363
@jkasten2 jkasten2 requested a review from itrush June 12, 2018 00:05
@itrush
Copy link
Contributor

itrush commented Jun 12, 2018

:lgtm:


Reviewed 2 of 2 files at r1.
Review status: all files reviewed, 1 unresolved discussion (waiting on @jkasten2)


test/unit/public-sdk-apis/showHttpPrompt.ts, line 24 at r1 (raw file):

  sinonSandbox.stub(MainHelper, "wasHttpsNativePromptDismissed").resolves(true);
  sinonSandbox.stub(OneSignal, "privateIsPushNotificationsEnabled").resolves(false);

Just a note, not blocking.

Even though this method is called privateIsPushNotificationsEnabled, it's visibility in OneSignal class is default, i.e. public. If it was real private member, you wouldn't be able to stub it. The naming is a bit confusing. We may add a note and refactor it and similar methods in the future.


Comments from Reviewable

@jkasten2
Copy link
Member Author

Review status: all files reviewed, 1 unresolved discussion (waiting on @jkasten2)


test/unit/public-sdk-apis/showHttpPrompt.ts, line 24 at r1 (raw file):

Previously, itrush (Iryna Trush) wrote…

Just a note, not blocking.

Even though this method is called privateIsPushNotificationsEnabled, it's visibility in OneSignal class is default, i.e. public. If it was real private member, you wouldn't be able to stub it. The naming is a bit confusing. We may add a note and refactor it and similar methods in the future.

@itrush Good catch, there are a few of these so we should clean them all up in another PR


Comments from Reviewable

@jkasten2 jkasten2 merged commit d2275ee into master Jun 12, 2018
@jkasten2 jkasten2 deleted the fix_private_show_http_prompt branch June 12, 2018 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Cannot read property 'force' of undefined
2 participants