search

PeriodPals / periodpals

3 stars 1 forks source link

feat/profile/integrate-vm-profile : Integrate profile VM into Profile screen #162

Closed charliemangano closed 3 weeks ago

charliemangano commented 3 weeks ago

Integrate profile VM into Profile screen

Description

This PR introduces syncing between our database and the Profile screen. Now, users can see the data they entered when singing up/editing their profile. It closes issue #160 and part of #154.

Changes

Replaced placeholder values for profile picture, name and description in Profile screen with data fetched from the database using UserViewModel.loadUser() and UserViewModel.user. In case of the view model failing to retrieve the user's data, display "Error loading X" instead.

Added a UserViewModel parameter in the ProfileScreen that is passed from the MainActivity.

Fixed a test tag that was on the wrong component.

Files

Added

None

Modified

Removed

None

Dependencies Added

None

Testing

Updated the existing tests to mock the user view model. They check the values displayed against those returned by the view model, and test for the view model failure case. Edit: as discussed in the comments with @agonzalez-r, I could not find a way to check the behavior of the profile picture. The GlideImage component seems to still be in development so maybe there is actually no way to do it yet.

charliemangano commented 3 weeks ago

@lazarinibruno

I don't know if it is common practice to do so for composables, but maybe adding a brief documentation of what every composable does would be a good idea? Or maybe not, as the names of the composables explain all that needs explanation? What do you think?

I agree! It's a bit outside of the scope of this PR but I'll do it as it's just docs.

charliemangano commented 3 weeks ago

@lazarinibruno I re-requested your review as merging from main introduced quite consequent changes in the code

sonarcloud[bot] commented 3 weeks ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarCloud

lazarinibruno commented 3 weeks ago

@lazarinibruno

I don't know if it is common practice to do so for composables, but maybe adding a brief documentation of what every composable does would be a good idea? Or maybe not, as the names of the composables explain all that needs explanation? What do you think?

I agree! It's a bit outside of the scope of this PR but I'll do it as it's just docs.

True! It does not really fit into this PR's goal. Thank you anyways for taking the time and doing it.