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

Possible fix for the join room screen not updating #3690

Merged

Conversation

Velin92
Copy link
Member

@Velin92 Velin92 commented Jan 21, 2025

Not sure if this is the best way to fix the issue given the API change to the room preview API that has been made, but once the request was sent, the state of the joined room was not updated, since the room preview was checked first but never updated.
Example:

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-21.at.11.26.11.mp4

This should however fix the issue:

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-01-21.at.11.43.52.mp4

@Velin92 Velin92 requested a review from a team as a code owner January 21, 2025 10:47
@Velin92 Velin92 requested review from pixlwave and stefanceriu and removed request for a team January 21, 2025 10:47
@Velin92
Copy link
Member Author

Velin92 commented Jan 21, 2025

I wonder if we shouldn't switch around this code:

        switch roomPreview.info.membership {
          case .invited:
              state.mode = .invited
          case .knocked:
              state.mode = .knocked
          case .banned:
              state.mode = .banned(sender: membershipDetails?.senderRoomMember?.displayName ?? membershipDetails?.senderRoomMember?.userID,
                                   reason: membershipDetails?.ownRoomMember.membershipChangeReason)
          default:
              switch roomPreview.info.joinRule {
              case .private, .invite:
                  state.mode = .inviteRequired
              case .knock, .knockRestricted:
                  state.mode = appSettings.knockingEnabled ? .knockable : .joinable
              case .restricted:
                  state.mode = .restricted
              default:
                  state.mode = .joinable
              }
          }
      } else if let room {
          switch room {
          case .invited:
              state.mode = .invited
          case .knocked:
              state.mode = .knocked
          case .banned:
              state.mode = .banned(sender: nil, reason: nil)
          default:
              state.mode = .joinable
          }
      }
  }

So that the room takes priority over the room preview, however I already tried this, and what happens is that the preview won't render the knockable state if I do so, so what I did for now is just also refresh the room preview after we refresh the room, when a change has been made @stefanceriu

@Velin92 Velin92 added the pr-bugfix for bug fix label Jan 21, 2025
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.58%. Comparing base (8577f53) to head (cbcef1f).
Report is 1 commits behind head on develop.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...reens/JoinRoomScreen/JoinRoomScreenViewModel.swift 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3690      +/-   ##
===========================================
- Coverage    78.66%   78.58%   -0.08%     
===========================================
  Files          792      792              
  Lines        67700    67700              
===========================================
- Hits         53253    53201      -52     
- Misses       14447    14499      +52     
Flag Coverage Δ
unittests 70.12% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pixlwave pixlwave removed their request for review January 21, 2025 12:21
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Velin92 Velin92 enabled auto-merge (squash) January 21, 2025 13:39
@Velin92 Velin92 merged commit 19c14d5 into develop Jan 21, 2025
8 checks passed
@Velin92 Velin92 deleted the mauroromito/possible_fix_for_sending_request_not_updating branch January 21, 2025 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix for bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants