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

Fix sync init when thread unread notif is not supported #2739

Merged
merged 10 commits into from
Oct 7, 2022

Conversation

germain-gg
Copy link
Contributor

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

Fixes element-hq/element-web#23435

Creates a solution to detect features available on a given homeserver.
Developers can now document what matrix version will support a feature as well as the unstable prefixes. This code path is then accessible synchronously

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:

🐛 Bug Fixes

return new Filter(this.client.credentials.userId);
const filter = new Filter(this.client.credentials.userId);
if (this.client.canSupport.get(Feature.ThreadUnreadNotifications)) {
filter.setUnreadThreadNotifications(true);
Copy link
Member

Choose a reason for hiding this comment

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

setUnreadThreadNotifications only uses the stable non-prefixed version but this.client.canSupport.get(Feature.ThreadUnreadNotifications) will return true if unstable is supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I've changed the code to use UnstableValue.
I believe I had not done that in the first place after seeing how setProp works... it essentially treats . in property names as a nested level

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should update setProp to take a variadic list of keys rather than a string to avoid this problem

src/store/memory.ts Outdated Show resolved Hide resolved
src/filter.ts Show resolved Hide resolved
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 to me! Any chance of an extra test around /sync actually including this filter?

src/feature.ts Outdated Show resolved Hide resolved
@germain-gg germain-gg enabled auto-merge (squash) October 7, 2022 10:33
@germain-gg germain-gg merged commit 62007ec into develop Oct 7, 2022
@germain-gg germain-gg deleted the gsouquet/thread-unread-notif-sync-fix branch October 7, 2022 10:38
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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error on initial sync and initial sync filter discarded
2 participants