Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

MSC3786: Add a default push rule to ignore m.room.server_acl events #2333

Merged
merged 6 commits into from
May 4, 2022

Conversation

SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Apr 29, 2022

Fixes element-hq/element-web#20788
Implements matrix-org/matrix-spec-proposals#3786
Synapse part to this: matrix-org/synapse#12601
Type: defect


Here's what your changelog entry will look like:

🐛 Bug Fixes

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2022

Codecov Report

Merging #2333 (62c72c8) into develop (2ebf335) will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2333      +/-   ##
===========================================
+ Coverage    59.76%   59.78%   +0.02%     
===========================================
  Files           91       91              
  Lines        16456    16456              
  Branches      3801     3801              
===========================================
+ Hits          9835     9839       +4     
+ Misses        6621     6617       -4     
Impacted Files Coverage Δ
src/pushprocessor.ts 74.00% <ø> (+2.00%) ⬆️

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@t3chguy
Copy link
Member

t3chguy commented Apr 30, 2022

I think the client has to more actively create the rule, otherwise client & server will be applying different push rules causing the notification_count being wrong

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner changed the title Don't notify on room server ACL changes MSC3786: Add a default push rule to ignore m.room.server_acl events Apr 30, 2022
@SimonBrandner SimonBrandner requested a review from t3chguy April 30, 2022 15:40
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner requested a review from t3chguy May 3, 2022 16:34
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

Looks sane

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
@SimonBrandner SimonBrandner merged commit 1cde686 into develop May 4, 2022
@SimonBrandner SimonBrandner deleted the SimonBrandner/fix/acl-notifs branch May 4, 2022 14:22
@t3chguy
Copy link
Member

t3chguy commented May 4, 2022

Hmm @SimonBrandner this says Requires https://github.com/matrix-org/synapse/pull/12601 yet landed before it?

@SimonBrandner
Copy link
Contributor Author

Hmm @SimonBrandner this says Requires https://github.com/matrix-org/synapse/pull/12601 yet landed before it?

That wasn't phrased well... It doesn't require it, it will just work better with it landed as we discussed, is it ok that I have merged the PR?

@t3chguy
Copy link
Member

t3chguy commented May 4, 2022

Just double checking that you didn't jump the gun, merge timing is all up to you :)

su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request May 28, 2022
* Implement changes to MSC2285 (private read receipts) ([\matrix-org#2221](matrix-org#2221)).
* Add support for HTML renderings of room topics ([\matrix-org#2272](matrix-org#2272)).
* Add stopClient parameter to MatrixClient::logout ([\matrix-org#2367](matrix-org#2367)).
* registration: add function to re-request email token ([\matrix-org#2357](matrix-org#2357)).
* Remove hacky custom status feature ([\matrix-org#2350](matrix-org#2350)).
* Remove default push rule override for MSC1930 ([\matrix-org#2376](matrix-org#2376)). Fixes element-hq/element-web#15439.
* Tweak thread creation & event adding to fix bugs around relations ([\matrix-org#2369](matrix-org#2369)). Fixes element-hq/element-web#22162 and element-hq/element-web#22180.
* Prune both clear & wire content on redaction ([\matrix-org#2346](matrix-org#2346)). Fixes element-hq/element-web#21929.
* MSC3786: Add a default push rule to ignore `m.room.server_acl` events ([\matrix-org#2333](matrix-org#2333)). Fixes element-hq/element-web#20788.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACL changes appear to be causing notifications in rooms
3 participants