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

[Poll] Synchronize polls push rules with message push rules (PSG-954) #8114

Merged
merged 9 commits into from
Feb 17, 2023

Conversation

Florian14
Copy link
Contributor

@Florian14 Florian14 commented Feb 10, 2023

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Add the new push rules for the polls related to the MSC3930.
Bind the new rules in the notification settings with the existing rules for direct messages and group messages.

Also:

  • deleted unused advanced notifications fragment
  • renamed notification settings classes (removed Preference from the class names)

Motivation and context

Close #8007

Note: the error handling and the initial sync of the rules when they are received from the server will be done in dedicated PRs.

Screenshots / GIFs

Tests

Needs a server supporting the new MSC for testing

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@Florian14 Florian14 requested review from a team, jmartinesp and jonnyandrew and removed request for a team and jmartinesp February 10, 2023 16:06
@Florian14 Florian14 force-pushed the feature/fre/poll_new_push_rules branch from d55b281 to 0a0ad2d Compare February 14, 2023 16:54
@ElementBot
Copy link

Warnings
⚠️

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt#L83 - It will always be more efficient to use more specific change events if you can. Rely on notifyDataSetChanged as a last resort.

⚠️

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt#L83 - It will always be more efficient to use more specific change events if you can. Rely on notifyDataSetChanged as a last resort.

⚠️

vector/src/main/java/im/vector/app/features/settings/notifications/advanced/VectorSettingsAdvancedNotificationPreferenceFragment.kt#L95 - It will always be more efficient to use more specific change events if you can. Rely on notifyDataSetChanged as a last resort.

⚠️

vector/src/main/java/im/vector/app/features/settings/notifications/advanced/VectorSettingsAdvancedNotificationPreferenceFragment.kt#L95 - It will always be more efficient to use more specific change events if you can. Rely on notifyDataSetChanged as a last resort.

Generated by 🚫 dangerJS against 0a0ad2d

@sonarcloud
Copy link

sonarcloud bot commented Feb 14, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

32.9% 32.9% Coverage
0.0% 0.0% Duplication

import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.pushrules.RuleIds
import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind

// TODO This fragment seems not used anymore, we can probably delete it
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth doing this now while we're already making changes to notification preference screens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it initially but it involved also removing the related resources. I preferred to revert to not include more changes in the PR and I was wondering if it could be used in f-droid (even if I did not find any usage). Wdyt @bmarty? If we can remove it, I'll handle it in a separate PR.

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.

[Poll] Synchronize the new push rules on general messages rules
3 participants