GT-1719 favorites drag and drop #2509

rachaelblue · 2025-03-25T00:45:51Z

Hey @levieggertcru, the one thing that's bugging me with this is that to move a tool you have to long press on it, but the tool background highlights when you do that. Do you think it looks weird? I feel like only the card itself should move and the background shouldn't do anything.

codecov · 2025-03-25T01:07:19Z

Codecov Report

Attention: Patch coverage is 77.66180% with 107 lines in your changes missing coverage. Please review.

Project coverage is 40.18%. Comparing base (9f2e517) to head (16f1f81).
Report is 35 commits behind head on develop.

Files with missing lines	Patch %	Lines
...llYourFavoriteTools/AllYourFavoriteToolsView.swift	0.00%	36 Missing ⚠️
...epository/Cache/RealmFavoritedResourcesCache.swift	78.57%	27 Missing ⚠️
...rFavoriteTools/AllYourFavoriteToolsViewModel.swift	0.00%	25 Missing ⚠️
.../Domain/UseCases/ReorderFavoritedToolUseCase.swift	0.00%	7 Missing ⚠️
...encyContainer/FavoritesDataLayerDependencies.swift	0.00%	5 Missing ⚠️
...cyContainer/FavoritesDomainLayerDependencies.swift	0.00%	5 Missing ⚠️
godtools/App/Flows/App/AppFlow.swift	0.00%	1 Missing ⚠️
...sourcesRepository/FavoritedResourceDataModel.swift	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2509      +/-   ##
===========================================
+ Coverage    39.82%   40.18%   +0.36%     
===========================================
  Files         1144     1151       +7     
  Lines        58086    58467     +381     
===========================================
+ Hits         23131    23495     +364     
- Misses       34955    34972      +17

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

levieggertcru

Hey @rachaelblue looking good so far! You hit the architecture points really well and I also like the tests you're including.
I did see one issue and I think it has to do with adding the Realm position 0 to start. Added more on that in the Review comments.

...App/Features/Favorites/Presentation/AllYourFavoriteTools/AllYourFavoriteToolsViewModel.swift

godtools/App/Share/Data/FavoritedResourcesRepository/Cache/Models/RealmFavoritedResource.swift

...App/Features/Favorites/Presentation/AllYourFavoriteTools/AllYourFavoriteToolsViewModel.swift

levieggertcru · 2025-03-25T14:30:34Z

Hey @rachaelblue I think it would be nice if we could exclude the highlight background from the drag and drop, however, I'm not sure how difficult it may be to exclude that? I'm curious to look at that some more in the Xcode UI debugger too so will look more at that.

godtools/App/Features/Dashboard/Presentation/Tools/Subviews/ToolCard/ToolCardView.swift

levieggertcru · 2025-03-25T14:37:50Z

One more thing, can you remove this line at the top of the List? Hopefully that is easy to remove.

...sTests/App/Features/Favorites/Data-DomainInterface/ReorderFavoritedToolRepositoryTests.swift

godtools/App/Features/Favorites/Domain/UseCases/ReorderFavoritedToolUseCase.swift

…into GT-1719-Favorites-Drag-and-Drop

rachaelblue · 2025-04-01T19:09:33Z

Hey @levieggertcru, I think this is ready for a second look.

…-changes GT-1719 revert tool card view changes

levieggertcru

Hey @rachaelblue this is looking really nice! Sorting is running smooth.

I did notice a couple things. One with the sorting logic on storage of new favorites. See my comment there and let me know your thoughts on option 2. I created a PR to see what I'm referring to. (#2529)

Also with the validation, moving that method call out of the publisher. This actually might help with option 2 when storing new favorited tools.

...ools/App/Features/Favorites/Presentation/AllYourFavoriteTools/AllYourFavoriteToolsView.swift

...App/Features/Favorites/Presentation/AllYourFavoriteTools/AllYourFavoriteToolsViewModel.swift

godtools/App/Share/Data/FavoritedResourcesRepository/Cache/RealmFavoritedResourcesCache.swift

This reverts commit 611bd82.

…into GT-1719-Favorites-Drag-and-Drop

…iling-test-in-toggle-tool-favorited-repository-tests

…n-toggle-tool-favorited-repository-tests GT-1719 create failing test in toggle tool favorited repository tests

GT-1719 remove invalid test

…ort by createdAt

rachaelblue · 2025-04-10T19:34:32Z

Hey @levieggertcru, made those changes we discussed and ready for review.

levieggertcru · 2025-04-10T21:10:23Z

Thanks @rachaelblue !

levieggertcru

Thanks @rachaelblue, I like this direction better. I like that the createdAt is removed too.

Just 2 little things I noticed.

levieggertcru · 2025-04-10T21:12:48Z

godtools/App/Share/Data/FavoritedResourcesRepository/Cache/RealmFavoritedResourcesCache.swift

+
+            let favoritesSortedByCreatedAt = realm
+                .objects(RealmFavoritedResource.self)
+                .sorted(byKeyPath: #keyPath(RealmFavoritedResource.createdAt), ascending: !ascendingOrder)


Hey @rachaelblue I ran a fresh build on develop, pre migration, and added a tool to my favorited list. I then ran this branch and I noticed my tool order was in reverse.

It took me a second to see why, but right here on ascendingOrder the flag is getting flipped !ascendingOrder .

I would suggest putting false there and removing the argument on the method. We will always want to migrate those so the most recent date is at position 0.

Then you will need to update your methods getFavoritedResourcesSortedByPosition and getFavoritedResourcesSortedByPositionPublisher to sort by position if you want to use the ascendingOrder argument on those methods.

Hey @levieggertcru, I went ahead and just removed ascendingOrder entirely. I think it made sense that you might want that flexibility when sorting by dates, but doesn't seem needed with positions. And it's error-prone with the migration, since ascending means opposite things depending on if you're sorting by position or date 🫣

Hey @rachaelblue that sounds good!

levieggertcru · 2025-04-10T21:14:32Z

...ols/App/Features/ToolShortcutLinks/Data-DomainInterface/GetToolShortcutLinksRepository.swift

@@ -25,7 +25,7 @@ class GetToolShortcutLinksRepository: GetToolShortcutLinksRepositoryInterface {

    func getLinksPublisher(appLanguage: AppLanguageDomainModel) -> AnyPublisher<[ToolShortcutLinkDomainModel], Never> {

-        return favoritedResourcesRepository.getFavoritedResourcesSortedByCreatedAtPublisher(ascendingOrder: false)
+        return favoritedResourcesRepository.getFavoritedResourcesSortedByPositionPublisher(ascendingOrder: false)


For tool shortcuts make sure it matches the favorited list order. Ascending defaults to true so you will want to remove this flag.

levieggertcru

Hey @rachaelblue looks good! Getting correct migration order.

rachaelblue added 8 commits March 21, 2025 13:43

allow favorited tools to be move tools around and create useCases

91e2f58

sort favorites by position

879b6d4

add test for reordering

05d79be

add second test

04deec3

fix off by one error when moving an item down

8aba919

fix all buttons triggering on list tap

3017023

adjust positions when favorite is removed

aeea2a2

fix failing tests

ee84694

rachaelblue marked this pull request as ready for review March 25, 2025 00:49

rachaelblue requested a review from levieggertcru March 25, 2025 00:49

levieggertcru requested changes Mar 25, 2025

View reviewed changes

levieggertcru reviewed Mar 25, 2025

View reviewed changes

godtools/App/Features/Dashboard/Presentation/Tools/Subviews/ToolCard/ToolCardView.swift Outdated Show resolved Hide resolved

levieggertcru reviewed Mar 25, 2025

View reviewed changes

...sTests/App/Features/Favorites/Data-DomainInterface/ReorderFavoritedToolRepositoryTests.swift Show resolved Hide resolved

levieggertcru reviewed Mar 25, 2025

View reviewed changes

godtools/App/Features/Favorites/Domain/UseCases/ReorderFavoritedToolUseCase.swift Outdated Show resolved Hide resolved

rachaelblue added 2 commits March 25, 2025 15:06

store cancellables in static var

37edaa2

validate favorites positions

bf6fa3f

rachaelblue marked this pull request as draft March 27, 2025 19:37

rachaelblue added 8 commits March 27, 2025 15:50

sort by createdAt when validating

cfcd1c7

Merge branch 'develop' of https://github.com/CruGlobal/godtools-swift …

f04c558

…into GT-1719-Favorites-Drag-and-Drop

add reorderFavorite domain model

98cb20a

add test for removingFavorite and checking position

a4019a2

fix failing test

e7a5edd

add test for adding a favorite tool

293f677

commit changes

0b7e8db

remove row separator from top of list

30fdfc2

rachaelblue marked this pull request as ready for review April 1, 2025 19:08

rachaelblue requested a review from levieggertcru April 1, 2025 19:09

levieggertcru and others added 3 commits April 2, 2025 10:25

Reverting changes to dev since nothing changed

3fb9dba

Merge pull request #2528 from CruGlobal/GT-1719-revert-tool-card-view…

2a839df

…-changes GT-1719 revert tool card view changes

Create failing test by adding all favorited tools at position 0.

8d63445

levieggertcru requested changes Apr 2, 2025

View reviewed changes

rachaelblue added 2 commits April 3, 2025 15:55

validate favorites after storing

611bd82

validate resources sorted by position not in publisher

4ce2e10

rachaelblue requested a review from levieggertcru April 3, 2025 19:58

rachaelblue marked this pull request as draft April 3, 2025 20:03

rachaelblue removed the request for review from levieggertcru April 3, 2025 20:05

rachaelblue added 3 commits April 5, 2025 14:26

Revert "validate favorites after storing"

5fa4611

This reverts commit 611bd82.

fix adding multiple favorites all having position 0 and write test

306dc33

Merge branch 'develop' of https://github.com/CruGlobal/godtools-swift …

e70f1ec

…into GT-1719-Favorites-Drag-and-Drop

rachaelblue requested a review from levieggertcru April 5, 2025 19:08

levieggertcru and others added 6 commits April 8, 2025 10:24

Merge branch 'GT-1719-Favorites-Drag-and-Drop' into GT-1719-create-fa…

94dc147

…iling-test-in-toggle-tool-favorited-repository-tests

Merge pull request #2529 from CruGlobal/GT-1719-create-failing-test-i…

d7a21dc

…n-toggle-tool-favorited-repository-tests GT-1719 create failing test in toggle tool favorited repository tests

Remove invalid test

98795c3

Merge pull request #2538 from CruGlobal/GT-1719-remove-invalid-test

cca0719

GT-1719 remove invalid test

return void from store favorites method

b478f91

rename validation method to migration and delete methods that fetch/s…

cb4d2fe

…ort by createdAt

rachaelblue marked this pull request as ready for review April 10, 2025 19:33

levieggertcru requested changes Apr 10, 2025

View reviewed changes

remove ascending order param

17f27ef

rachaelblue requested a review from levieggertcru April 11, 2025 20:05

levieggertcru approved these changes Apr 11, 2025

View reviewed changes

Merge branch 'develop' into GT-1719-Favorites-Drag-and-Drop

16f1f81

rachaelblue merged commit dd4d690 into develop Apr 14, 2025
4 checks passed

rachaelblue deleted the GT-1719-Favorites-Drag-and-Drop branch April 14, 2025 18:04

GT-1719 favorites drag and drop #2509

GT-1719 favorites drag and drop #2509

Uh oh!

Conversation

rachaelblue commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

levieggertcru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

levieggertcru commented Mar 25, 2025

Uh oh!

Uh oh!

levieggertcru commented Mar 25, 2025

Uh oh!

Uh oh!

Uh oh!

rachaelblue commented Apr 1, 2025

Uh oh!

levieggertcru left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rachaelblue commented Apr 10, 2025

Uh oh!

levieggertcru commented Apr 10, 2025

Uh oh!

levieggertcru left a comment

Choose a reason for hiding this comment

Uh oh!

levieggertcru Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

levieggertcru Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

levieggertcru Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

rachaelblue Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

levieggertcru Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

levieggertcru Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

levieggertcru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rachaelblue commented Mar 25, 2025 •

edited

Loading

codecov bot commented Mar 25, 2025 •

edited

Loading

levieggertcru left a comment •

edited

Loading

levieggertcru Apr 10, 2025 •

edited

Loading

levieggertcru Apr 10, 2025 •

edited

Loading