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.

Sign up for GitHub

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

Jump to bottom

fix: persistent audio state and observe senderId [WPB-4716] #2661

Merged
merged 10 commits into from
Feb 7, 2024
Merged

fix: persistent audio state and observe senderId [WPB-4716] #2661

merged 10 commits into from
Feb 7, 2024

Conversation

Garzas
Copy link
Contributor

@Garzas Garzas commented Feb 5, 2024 •
edited by github-actions bot
Loading

BugWPB-4716 [ANR] Janky scrolling in a conversation

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

  • we were using map instead of persistent map to provide message audio state.
  • we are observing unnecessary sender when it's available from message

Causes (Optional)

  • unnecessary compose rebuild when scrolling messages
  • more db executions to get sender user entity

Solutions

Also fixed crash when user want to edit deleted message

@Garzas Garzas self-assigned this Feb 5, 2024
@AndroidBob
Copy link
Collaborator

Build 2872 failed.

@AndroidBob
Copy link
Collaborator

Build 2873 failed.

@AndroidBob
Copy link
Collaborator

Build 2887 failed.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Feb 6, 2024
@Garzas Garzas requested a review from saleniuk February 6, 2024 12:40
@AndroidBob
Copy link
Collaborator

Build 2907 failed.

yamilmedina
yamilmedina reviewed Feb 6, 2024
View reviewed changes
Copy link
Contributor

@yamilmedina yamilmedina left a comment

Choose a reason for hiding this comment

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

Good catch ! I added a small question

app/src/main/kotlin/com/wire/android/ui/home/conversations/usecase/GetUsersForMessageUseCase.kt Show resolved Hide resolved
@yamilmedina yamilmedina self-requested a review February 6, 2024 15:24
@AndroidBob
Copy link
Collaborator

Build 2925 failed.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 7, 2024 •
edited
Loading

Test Results

791 tests   790 ✅  14m 32s ⏱️
108 suites    0 💤
108 files      1 ❌

For more details on these failures, see this check.

Results for commit 5571780.

♻️ This comment has been updated with latest results.

@AndroidBob
Copy link
Collaborator

Build 2938 failed.

@Garzas Garzas enabled auto-merge (squash) February 7, 2024 08:49
@AndroidBob
Copy link
Collaborator

Build 2939 failed.

@Garzas Garzas merged commit 9a2b353 into release/candidate Feb 7, 2024
11 of 12 checks passed
@Garzas Garzas deleted the fix/persistent-audio-state branch February 7, 2024 09:46
@AndroidBob AndroidBob mentioned this pull request Feb 7, 2024
Merged
4 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 7, 2024

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 2940 succeeded.

The build produced the following APK's:

@echoes-hq echoes-hq bot added the echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt. label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yamilmedina yamilmedina yamilmedina approved these changes

@saleniuk saleniuk saleniuk approved these changes

@MohamadJaara MohamadJaara Awaiting requested review from MohamadJaara

Assignees

@Garzas Garzas

Labels
echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Garzas @AndroidBob @yamilmedina @saleniuk