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 user completion for matrix ids #8271

Merged
merged 3 commits into from
Mar 30, 2023
Merged

Conversation

yostyle
Copy link
Contributor

@yostyle yostyle commented Mar 23, 2023

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Add user completion for matrix ids

Motivation and context

#8217

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@yostyle yostyle requested review from a team and Florian14 and removed request for a team March 23, 2023 13:54
Comment on lines 131 to 135
.filter { roomMemberSummary ->
query?.let {
it.isBlank() || roomMemberSummary.displayName?.contains(it, true).orFalse() || roomMemberSummary.userId.contains(it, true)
}.orTrue()
}
Copy link
Contributor

@Florian14 Florian14 Mar 24, 2023

Choose a reason for hiding this comment

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

I'm wondering if it's better to add the filter on the resulting list or in the realm query, something like this should work:

    override fun getRoomMembers(queryParams: RoomMemberQueryParams, query: String): List<RoomMemberSummary> {
        return monarchy.fetchAllMappedSync(
                {
                    roomMembersQuery(it, queryParams)
                            .and()
                            .contains(RoomMemberSummaryEntityFields.USER_ID, query)
                            .or()
                            .contains(RoomMemberSummaryEntityFields.DISPLAY_NAME, query)
                },
                {
                    it.asDomain()
                }
        )
    }

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add val displayNameOrUserId: QueryStringValue, to the data class RoomMemberQueryParams?

@sonarcloud
Copy link

sonarcloud bot commented Mar 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

22.7% 22.7% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@Florian14 Florian14 left a comment

Choose a reason for hiding this comment

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

Changes LGTM

@yostyle yostyle merged commit ed121c0 into develop Mar 30, 2023
@yostyle yostyle deleted the yostyle/autocomplete_userid branch March 30, 2023 08:05
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.

3 participants