PeriodPals / periodpals

3 stars 1 forks source link

Fix/profile/sync-problems : Fix sync problem with CreateProfile screen #191

Closed charliemangano closed 2 weeks ago

charliemangano commented 2 weeks ago

Fix sync problem with CreateProfile screen

Description

This PR introduces a fix for issue #190: it solves the syncing problem that made a user have to click two times on the "Save" button on the CreateProfile screen.

The problem came from how we handle our side effects, as the callbacks of the view model execute, at least partly, only after we've checked in the CreateProfile screen that the saving had succeeded, for example:

// in `CreateProfile`
userViewModel.saveUser()              // this does not finish executing

if (userState.value == null) {...}    // before this happens

So our CreateProfile screen thought that the operation failed, when it had just not finished executing.

Changes

Changed the signature of the functions in UserViewModel to have onSuccess and onFailure callbacks. I moved the code that was previously in the button's onClick into the callbacks of the calls to saveUser and loadUser. This ensures that this bit of code is executed after the rest of the saveUser or loadUser code, which solves the problem. These callback all have simple Logs as default values. Docs were updated as well. Updated the tests for the CreateProfile screen to implement the new callbacks properly.

Files

Added

None

Modified

Removed

None

Dependencies Added

None

Testing

Existing tests were updated but still check for the same things. Tests all pass. None more written. This should help for the end-to-end tests.

Edit: 14/11 8:22PM

Following a merge (24910ae), the function I was modifying (onClick of the "Save" Button of CreateProfile) got refactored into a different file (ProfileComponents.kt) that was now shared with EditProfile. This means that any modification I made to fix CreateProfile's onClick, I now had to do in the EditProfile screen as well, and the associated tests. This PR therefore got extended after it had been approved. Having now modified everything I needed to, I'm requesting a re-review.

Updated list of modifications: Everything from above, plus implementing changes asked in reviews, plus:

sonarcloud[bot] commented 2 weeks ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
80.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

charliemangano commented 2 weeks ago

Merging this but we will definitely create a task to see what to do about the dispatchers @coaguila. Thanks for the last minute review!