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.

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

[user model] Remove getPermissionStatus & add permission property #1035

Merged
merged 9 commits into from
May 11, 2023

Conversation

rgomezp
Copy link
Contributor

@rgomezp rgomezp commented May 10, 2023

Summary

This PR updates the SDK by removing the getPermissionStatus() function, adding a permission property, moving functions to the PermissionManager, renaming and moving files to improve organization, and adding test coverage for proper functionality.

Overview

This PR makes several updates to the SDK to improve organization and functionality. The getPermissionStatus() function has been removed, and a permission property has been added for clearer and more concise permission status checks. The getPermissionStatus() function has been moved to the PermissionManager to avoid cluttering the SDK with redundant functions. The helpers.ts file has been renamed and moved to a helpers directory for better organization, and test coverage has been added to ensure proper functionality of the SDK.

Details

  • Remove getPermissionStatus() function and add permission property to SDK. The getPermissionStatus() function was redundant and not in line with other SDKs, and adding the permission property provides a clearer way to check the permission status.
  • Move getPermissionStatus() to PermissionManager. This is done to better organize the code and avoid cluttering the SDK with redundant functions.
  • Update getPermissionStatus() function and fix usages throughout SDK. This is done to ensure that the SDK is compatible with other SDKs and to avoid breaking existing usage.
  • Rename helpers.ts and move to helpers directory. This is done to better organize the code and avoid cluttering the root directory with helper files.
  • Add test coverage. This is done to ensure proper functionality of the SDK and to catch any potential bugs or issues that may arise when the native permission fires.

This change is Reviewable

@rgomezp rgomezp requested a review from jkasten2 May 10, 2023 20:31
rgomezp added 3 commits May 10, 2023 16:06
Motivation:
* remove `getPermissionStatus` (redundant and not in line with other SDKs)
* add `permission` => boolean
Motivation: in order to not break existing usage, we will keep it around in the `PermissionManager`
@rgomezp rgomezp force-pushed the user-model/fix-notification-permission-api branch from 7af8837 to 693eedb Compare May 10, 2023 21:06
rgomezp added 3 commits May 10, 2023 17:24
Motivation: check that the properties change when the native permission fires
@rgomezp rgomezp changed the title Remove getPermissionStatus & add permission property [user model] Remove getPermissionStatus & add permission property May 10, 2023
@rgomezp rgomezp requested a review from jkasten2 May 11, 2023 18:31
@rgomezp rgomezp merged commit af0ddbf into user-model/v1 May 11, 2023
@rgomezp rgomezp deleted the user-model/fix-notification-permission-api branch May 11, 2023 18:33
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.

2 participants