Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Suggest the server's results as lower quality in the invite dialog #4149

Merged
merged 1 commit into from
Mar 3, 2020

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Feb 28, 2020

This is a quick win for fixing element-hq/element-web#12488 but might not be a long-term solution. Idea is to see how this feels and go from there, which may mean scoring the results again to filter them in or altering the debounce timers.

Fixes element-hq/element-web#12488

This is a quick win for fixing element-hq/element-web#12488 but might not be a long-term solution. Idea is to see how this feels and go from there, which may mean scoring the results again to filter them in or altering the debounce timers.
@turt2live turt2live requested a review from a team February 28, 2020 21:00
@turt2live turt2live changed the title Suggest the server's results as lower quality Suggest the server's results as lower quality in the invite dialog Feb 28, 2020
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

otherwise looks plausible

const hasMixins = this.state.serverResultsMixin || this.state.threepidResultsMixin;
if (this.state.filterText && hasMixins && kind === 'suggestions') {
// We don't want to duplicate members though, so just exclude anyone we've already seen.
const notAlreadyExists = (u: Member): boolean => {
return !sourceMembers.some(m => m.userId === u.userId)
&& !additionalMembers.some(m => m.userId === u.userId);
&& !priorityAdditionalMembers.some(m => m.userId === u.userId)
&& !otherAdditionalMembers.some(m => m.userId === u.userId);
Copy link
Member

Choose a reason for hiding this comment

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

These are just declared above the if block - I'm not seeing when they'd ever be non-empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sanity more than anything, the otherAdditionalMembers array would be populated when priorityAdditionalMembers is defined. To prevent future bugs, we just check both much like we were doing before.

Copy link
Member

Choose a reason for hiding this comment

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

fair enough!

@turt2live turt2live requested a review from dbkr March 3, 2020 00:29
@turt2live turt2live merged commit a50e4f6 into develop Mar 3, 2020
@turt2live turt2live deleted the travis/fix-directory-results branch March 3, 2020 17:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New invite dialog flashes up (correct) results and then replaces them with incorrect ones.
2 participants