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

Improvement: Only show relevant push notification settings #452

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

FelberMartin
Copy link
Collaborator

Problem Description

In Android, we displayed all notification settings as toggles in the notifications settings page, even if some of them should not support push notifications, or are not available to the user because of their role (eg students could change settings for "Instructor Notifications")

This PR closes #439

Changes

  • Hide notifications that are not relevant for push notifications (ssh, vcs-token, data-export)
  • Hide notifications based on the user's authorities
  • Extracted localization into PushNotificationLocalization
  • Extracted filtering logic into new PushNotificationSettingUtil
  • Added tests

Steps for testing

  1. Log in as a student and see that only relevant notification settings are shown
  2. Log in as a tutor/editor/instructor and see that the authority based notifications are shown

@FelberMartin FelberMartin self-assigned this Mar 1, 2025
@FelberMartin FelberMartin added the ready for review This PR can be reviewed label Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR can be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support new notifications types
1 participant