Skip to content

[MBL-19897][All] Fix inbox list not refreshing after screen rotation on details screen#3633

Merged
kristofnemere merged 1 commit intomasterfrom
MBL-19897-fix-inbox-rotation-refresh
Apr 8, 2026
Merged

[MBL-19897][All] Fix inbox list not refreshing after screen rotation on details screen#3633
kristofnemere merged 1 commit intomasterfrom
MBL-19897-fix-inbox-rotation-refresh

Conversation

@kristofnemere
Copy link
Copy Markdown
Contributor

Test plan:

  1. Open any app (Parent app is most reproducible)
  2. Navigate to Inbox
  3. Open a conversation (details screen)
  4. Rotate the device
  5. Perform an inbox operation (e.g. archive, mark as read/unread)
  6. Navigate back to the inbox list
  7. Verify the list reflects the change

refs: MBL-19897
affects: Student, Teacher, Parent
release note: Fixed inbox list not updating after performing operations on the conversation details screen following a screen rotation.

  • Follow-up e2e test ticket created or not needed
  • Tested in dark mode
  • Tested in light mode
  • Test in landscape mode and/or tablet
  • A11y checked
  • Approve from product

…on details screen

Move sharedEvents subscription from onViewCreated to onCreate so the fragment
is always listening even when its view hasn't been recreated yet (back stack
lazy view inflation after rotation).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Review Summary

This PR fixes a legitimate bug in InboxFragment where calling lifecycleScope.collectOneOffEvents(sharedEvents.events, …) inside onViewCreated could create duplicate subscriptions each time the fragment's view was destroyed and recreated (e.g., returning from the details screen via back-press). Since lifecycleScope is bound to the Fragment lifecycle (not the View lifecycle), the coroutine launched on each onViewCreated call was never cancelled on view destruction, resulting in N concurrent subscribers after N view recreations.

Moving the subscription to onCreate is the correct fix — it is called exactly once per Fragment instance, ensuring a single subscription for the lifetime of the fragment.

The handler handleSharedViewModelAction only calls viewModel.invalidateCache() and viewModel.refresh() — pure ViewModel operations with no dependency on the View — so it is safe to start collecting before onViewCreated.


Issues Found

  • Same bug exists in InboxDetailsFragmentlibs/pandautils/src/main/java/com/instructure/pandautils/features/inbox/details/InboxDetailsFragment.kt, line 79

    // onCreateView — called every time the view is (re)created
    lifecycleScope.collectOneOffEvents(sharedEvents.events, ::handleSharedViewModelAction)

    InboxDetailsFragment still subscribes to sharedEvents.events via lifecycleScope inside onCreateView. If that fragment's view is ever recreated while the fragment instance survives (e.g., configuration change with retainInstance, or back-stack behaviour depending on navigation setup), duplicate subscriptions will accumulate in the same way. Consider moving this call to an onCreate override in InboxDetailsFragment as well, consistent with the fix applied here.


Overall: The change is minimal, targeted, and correct. No concerns with the modified code itself.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

🧪 Unit Test Results

✅ 📱 Parent App

  • Tests: 315 total, 0 failed, 0 skipped
  • Duration: 31.840s
  • Success Rate: 100%

✅ 📱 Student App

  • Tests: 1252 total, 0 failed, 0 skipped
  • Duration: 0.000s
  • Success Rate: 100%

✅ 📱 Teacher App

  • Tests: 373 total, 0 failed, 0 skipped
  • Duration: 28.655s
  • Success Rate: 100%

✅ 🌅 Horizon

  • Tests: 790 total, 0 failed, 0 skipped
  • Duration: 34.207s
  • Success Rate: 100%

✅ 📦 Submodules

  • Tests: 3349 total, 0 failed, 0 skipped
  • Duration: 55.062s
  • Success Rate: 100%

📊 Summary

  • Total Tests: 6079
  • Failed: 0
  • Skipped: 0
  • Status: ✅ All tests passed!

Last updated: Wed, 08 Apr 2026 07:38:55 GMT

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

📊 Code Coverage Report

✅ Student

  • PR Coverage: 42.65%
  • Master Coverage: 42.65%
  • Delta: +0.00%

✅ Teacher

  • PR Coverage: 25.37%
  • Master Coverage: 25.37%
  • Delta: +0.00%

✅ Pandautils

  • PR Coverage: 23.69%
  • Master Coverage: 23.69%
  • Delta: +0.00%

📈 Overall Average

  • PR Coverage: 30.57%
  • Master Coverage: 30.57%
  • Delta: +0.00%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

Parent Install Page

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

Teacher Install Page

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

Student Install Page

Copy link
Copy Markdown
Contributor

@tamaskozmer tamaskozmer left a comment

Choose a reason for hiding this comment

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

QA + 1

Copy link
Copy Markdown
Contributor

@adamNagy56 adamNagy56 left a comment

Choose a reason for hiding this comment

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

QA +1

@kristofnemere kristofnemere merged commit a779485 into master Apr 8, 2026
51 of 52 checks passed
@kristofnemere kristofnemere deleted the MBL-19897-fix-inbox-rotation-refresh branch April 8, 2026 13:29
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