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

Add support for unread thread notifications #2726

Merged
merged 13 commits into from
Oct 5, 2022

Conversation

germain-gg
Copy link
Contributor

@germain-gg germain-gg commented Oct 3, 2022

Implements matrix-org/matrix-spec-proposals#3773
Related to element-hq/element-web#23192

Passes the quality gate but still has optional strict null checks failures

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

✨ Features

  • Add support for unread thread notifications (#2726).

@germain-gg germain-gg marked this pull request as ready for review October 4, 2022 10:29
@germain-gg germain-gg requested a review from a team as a code owner October 4, 2022 10:29
Copy link
Contributor

@weeman1337 weeman1337 left a comment

Choose a reason for hiding this comment

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

👍 just some small questions

src/sync.ts Outdated
@@ -1264,6 +1268,28 @@ export class SyncApi {
}
}

room.resetThreadUnreadNotificationCount();
const unreadThreadNotifications = joinObj[UNREAD_THREAD_NOTIFICATIONS.name];
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we try to retrieve the value by altName as well?

src/sync.ts Show resolved Hide resolved
@germain-gg germain-gg requested review from weeman1337 and removed request for florianduros October 5, 2022 08:17
@germain-gg germain-gg merged commit 21a6f61 into develop Oct 5, 2022
@germain-gg germain-gg deleted the gsouquet/notifications-msc3773 branch October 5, 2022 09:37
@t3chguy
Copy link
Member

t3chguy commented Oct 6, 2022

This breaks initialSync filtering support

21a6f61#diff-1b0cb46cd1fcdaacf73c771bf7eed6e00a12ee030da1dbcca4ae12842e1ecd81R709-R712

image

This may need reverting if not remedied along with tests

@t3chguy
Copy link
Member

t3chguy commented Oct 6, 2022

Inherently this is broken

  1. it never sets the unstable-prefixed filter
  2. it sets the filter with a value of false which is redundant
  3. it only sets it for the initial sync and not any later syncs which means the counts won't ever update on incremental/gappy syncs.

su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Oct 29, 2022
* Changes the `uploadContent` API, kills off `request` and `browser-request` in favour of `fetch`, removed callback support on a lot of the methods, adds a lot of tests. ([\matrix-org#2719](matrix-org#2719)). Fixes matrix-org#2415 and matrix-org#801.
* Remove deprecated `m.room.aliases` references ([\matrix-org#2759](matrix-org#2759)). Fixes element-hq/element-web#12680.
* Remove node-specific crypto bits, use Node 16's WebCrypto ([\matrix-org#2762](matrix-org#2762)). Fixes matrix-org#2760.
* Export types for MatrixEvent and Room emitted events, and make event handler map types stricter ([\matrix-org#2750](matrix-org#2750)). Contributed by @stas-demydiuk.
* Use even more stable calls to `/room_keys` ([\matrix-org#2746](matrix-org#2746)).
* Upgrade to Olm 3.2.13 which has been repackaged to support Node 18 ([\matrix-org#2744](matrix-org#2744)).
* Fix `power_level_content_override` type ([\matrix-org#2741](matrix-org#2741)).
* Add custom notification handling for MSC3401 call events  ([\matrix-org#2720](matrix-org#2720)).
* Add support for unread thread notifications ([\matrix-org#2726](matrix-org#2726)).
* Load Thread List with server-side assistance (MSC3856) ([\matrix-org#2602](matrix-org#2602)).
* Use stable calls to `/room_keys` ([\matrix-org#2729](matrix-org#2729)). Fixes element-hq/element-web#22839.
* Fix POST data not being passed for registerWithIdentityServer ([\matrix-org#2769](matrix-org#2769)). Fixes matrix-org/element-web-rageshakes#16206.
* Fix IdentityPrefix.V2 containing spurious `/api` ([\matrix-org#2761](matrix-org#2761)). Fixes element-hq/element-web#23505.
* Always send back an httpStatus property if one is known ([\matrix-org#2753](matrix-org#2753)).
* Check for AbortError, not any generic connection error, to avoid tightlooping ([\matrix-org#2752](matrix-org#2752)).
* Correct the dir parameter of MSC3715 ([\matrix-org#2745](matrix-org#2745)). Contributed by @dhenneke.
* Fix sync init when thread unread notif is not supported ([\matrix-org#2739](matrix-org#2739)). Fixes element-hq/element-web#23435.
* Use the correct sender key when checking shared secret ([\matrix-org#2730](matrix-org#2730)). Fixes element-hq/element-web#23374.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants