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

Update the actions of default rules instead of overriding. #1037

Merged
merged 5 commits into from
Mar 15, 2016

Conversation

NegativeMjark
Copy link
Contributor

The Matrix CS API, and synapse now supports setting the actions for default
rules. Doing that makes managing the rules much simpler from a vector
persepctive since the ON/LOUD/OFF toggle buttons can be implemented by
setting the actions and enabling/disabling the default rules rather than
overidding them.

Overriding the default rules was difficult because it was not possible
to intermingle the evaluation of user-specified rules with the default
rules. So even though you could add a rule with the same conditions as a
default rule, it would evaluate before all the other default rules.

Also creating new rules under a im.vector namespace creates challenges
if we want vector to cooperate with other matrix clients that want to
provide a similar set of toggle switches for the push rules.

The Matrix CS API, and synapse now supports setting the actions for default
rules. Doing that makes managing the rules much simpler from a vector
persepctive since the ON/LOUD/OFF toggle buttons can be implemented by
setting the actions and enabling/disabling the default rules rather than
overidding them.

Overriding the default rules was difficult because it was not possible
to intermingle the evaluation of user-specified rules with the default
rules. So even though you could add a rule with the same conditions as a
default rule, it would evaluate before *all* the other default rules.

Also creating new rules under a im.vector namespace creates challenges
if we want vector to cooperate with other matrix clients that want to
provide a similar set of toggle switches for the push rules.
@NegativeMjark
Copy link
Contributor Author

I've added some code for when vector downloads the push rules to migrate from the "im.vector" rules to setting the actions for the default ".m.*" rules.

// "highlight: true/false,
// }
// If the actions couldn't be decoded then returns null.
function decodeActions(actions) {
Copy link
Member

Choose a reason for hiding this comment

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

This is this function: https://github.com/matrix-org/matrix-js-sdk/blob/develop/lib/pushprocessor.js#L227

We should probably think about exposing it in the js sdk rather than duplicating the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure if they are exactly the same function since this gives up if it doesn't understand the tweaks, or actions.

But I agree that we should think about moving some of this code into the SDK so that other client authors don't need to duplicate it.

@dbkr
Copy link
Member

dbkr commented Mar 15, 2016

otherwise looking good

@dbkr dbkr assigned NegativeMjark and unassigned dbkr Mar 15, 2016
@NegativeMjark NegativeMjark assigned dbkr and unassigned NegativeMjark Mar 15, 2016
@dbkr
Copy link
Member

dbkr commented Mar 15, 2016

lgtm then

@dbkr dbkr assigned NegativeMjark and unassigned dbkr Mar 15, 2016
NegativeMjark added a commit that referenced this pull request Mar 15, 2016
Update the actions of default rules instead of overriding.
@NegativeMjark NegativeMjark merged commit eb01cb9 into develop Mar 15, 2016
@t3chguy t3chguy deleted the markjh/change_push_actions branch May 12, 2022 08:54
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