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

Simplify JobMapComposables further by pulling logic out of view layers #3045

Merged
merged 13 commits into from
Feb 12, 2025

Conversation

shobhitagarwal1612
Copy link
Member

This PR aims at improving code health by pulling more complexity out of the composables.

Verified for regressions by running the app locally.

@anandwana001 PTAL?

* Remove redundant setCollectDataListener function.
* Move data collection logic to HomeScreenMapContainerFragment.
* Pass the onCollectData callback directly to the composables.
* Remove the collectDataListener variable.
* Add canUserSubmitData check in onCollectData method in the fragment.
…nData`

*   Removed `canUserSubmitData` from `JobMapComposables.updateData` and `LoiJobSheet`.
*   Moved `canUserSubmitData` state management to `AdHocDataCollectionButtonData`
* Updated `HomeScreenMapContainerFragment` to use `AdHocDataCollectionButtonData`'s flag to check data submission permission.
*   Updated `InitializeAddLoiButton` in `JobMapComposables` to check `canCollectData` in `AdHocDataCollectionButtonData` instead of using the old state.
* Updated `LoiJobSheet` in `JobMapComposables` to use `loiData.canCollectData` for data submission permission.
*   Remove `State` from `canUserSubmitDataState` parameter in `LoiJobSheet` composable.
*   Pass `canUserSubmitData` directly instead of `canUserSubmitDataState` to `LoiJobSheet` in `JobMapComposables`
* Update usages of `canUserSubmitDataState` in ModalContents preview and tests.
*   Refactored `InitializeJobCard` to accept an `onCollectClicked` callback instead of `onCollectData`.
*   Refactored `InitializeAddLoiButton` to accept an `onCollectData` callback.
*   Updated logic to start data collection for a single job directly when adding a location of interest (LOI).
*   Removed unused parameters in InitializeJobCard.
* Remove redundant condition to show/hide the add loi button.
* Ensure that the new LOI button is not shown if no job can collect data.
@auto-assign auto-assign bot requested a review from anandwana001 February 12, 2025 09:52
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 42.59259% with 31 lines in your changes missing coverage. Please review.

Project coverage is 62.89%. Comparing base (2c83072) to head (024381c).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...und/ui/home/mapcontainer/jobs/JobMapComposables.kt 36.36% 15 Missing and 6 partials ⚠️
...ome/mapcontainer/HomeScreenMapContainerFragment.kt 28.57% 3 Missing and 2 partials ⚠️
...id/ground/ui/home/mapcontainer/jobs/LoiJobSheet.kt 0.00% 4 Missing ⚠️
...me/mapcontainer/HomeScreenMapContainerViewModel.kt 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3045      +/-   ##
============================================
+ Coverage     62.73%   62.89%   +0.16%     
+ Complexity     1300     1298       -2     
============================================
  Files           278      278              
  Lines          6792     6768      -24     
  Branches        947      949       +2     
============================================
- Hits           4261     4257       -4     
+ Misses         1910     1890      -20     
  Partials        621      621              
Files with missing lines Coverage Δ
.../mapcontainer/jobs/DataCollectionEntryPointData.kt 100.00% <100.00%> (ø)
...me/mapcontainer/HomeScreenMapContainerViewModel.kt 56.96% <80.00%> (-0.38%) ⬇️
...id/ground/ui/home/mapcontainer/jobs/LoiJobSheet.kt 0.00% <0.00%> (ø)
...ome/mapcontainer/HomeScreenMapContainerFragment.kt 17.42% <28.57%> (-0.23%) ⬇️
...und/ui/home/mapcontainer/jobs/JobMapComposables.kt 28.16% <36.36%> (-6.40%) ⬇️

@shobhitagarwal1612 shobhitagarwal1612 changed the title Simplify JobMapComposables further Simplify JobMapComposables further by pulling logic out of view layers Feb 12, 2025
@shobhitagarwal1612 shobhitagarwal1612 merged commit cc4a828 into master Feb 12, 2025
5 checks passed
@shobhitagarwal1612 shobhitagarwal1612 deleted the ashobhit/map-composable-cleanup-new branch February 12, 2025 12:21
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.

2 participants