refactor: remove TreeTrackerDAO and split TreeTackerDAOTest (#1235) - #5
Open
marsof02 wants to merge 9 commits into
Open
refactor: remove TreeTrackerDAO and split TreeTackerDAOTest (#1235)#5marsof02 wants to merge 9 commits into
marsof02 wants to merge 9 commits into
Conversation
…reenstand#1307) Contributors (often AI-assisted) frequently open PRs without any visual proof of the change. This makes review slow and forces maintainers to manually request screenshots/recordings on nearly every PR. - Rework the PR template with a required 'Proof of change' section: a mandatory screen recording plus before/after screenshots for UI changes, and a 'no user-visible effect' escape-hatch checkbox. - Add a require-pr-media workflow that scans the PR description for an embedded video (always required) and a screenshot (required unless the PR is non-UI). When media is missing it upserts a friendly comment tagging the author and fails a status check so the PR cannot be merged. - Non-UI PRs (via the 'non-ui' label or the template checkbox) skip the screenshot requirement but still require a video. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…and#1310) Update the require-pr-media bot comment to state explicitly that a video is required even when there are no UI changes, so the affected parts of the app can be shown still working. Applies to the video bullet and both the non-UI and 'no user-visible effect' hint lines. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Fix GreenStand privacy policy capitalization * Update settings screenshot baseline --------- Co-authored-by: Jonathan Muller <elforama@gmail.com>
… release app (Greenstand#1312) Updates the project google-services.json from a fresh console download. This re-adds the .debug app and includes two newly registered apps (com.ftt.android.TreeTracker, .justdiggit) that are not yet wired to any build type. The fresh download did not include a .prerelease app registration, which would break the prerelease build (the Google Services plugin requires a matching package_name). Instead of registering a separate prerelease app, the prerelease client reuses the release app's mobilesdk_app_id and API key, so prerelease builds report Crashlytics/Analytics into the production Firebase app. This matches the intent of grouping prerelease with production. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* refactor: split TextButton.kt into multiple focused files for better organization * fix: restore throttled navigation and resolve ktlint issues after TextButton split (Greenstand#1237) * Clean up README.md by removing conflict markers Removed unnecessary merge conflict markers from README. --------- Co-authored-by: Aneesh Pallapolu <yaagnik.p@gmail.com>
* chore(deps): bump Compose to 1.10.5, compileSdk to 36, add Navigation 3 deps Prerequisite for the Navigation 2 -> Navigation 3 migration: navigation3-ui 1.1.1 requires Compose >= 1.10.0 and compileSdk 36. material-icons-extended stays at 1.7.8 (last published version). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(navigation): migrate from Navigation 2 to Navigation 3 Replace androidx.navigation:navigation-compose with androidx.navigation3: - Routes implement NavKey; back stack owned by the app via rememberNavBackStack - New Navigator + NavOptions DSL (popUpTo<T>, popUpToRoot, launchSingleTop) replacing NavHostController and NavigationUtils' throttled helpers - NavHost -> NavDisplay in Host.kt and ImageCaptureActivity, with saveable-state and ViewModel-store entry decorators and the same 500ms fade transitions - trackedComposable -> custom screen-tracking NavEntryDecorator (Crashlytics breadcrumbs); HandleUIEvents gains an origin-is-top guard replacing the Nav2 RESUMED check - Org deep link parsed manually from the launch intent (cold start only, matching previous behavior) - NavigatorTest + FakeNavigator; previews/screenshot harness on LocalNavigator Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(navigation): provide NavigationEventDispatcherOwner for NavDisplay NavDisplay's back handling crashed on launch: AppCompatActivity's setContentView (appcompat 1.7.x) plants only the pre-navigationevent view-tree owners, so nav3 could not find a NavigationEventDispatcher. - Bump androidx.activity to 1.12.0, where ComponentActivity implements NavigationEventDispatcherOwner - Plant the view-tree owner on the decor view in TreeTrackerActivity Verified on a Pixel 7 API 35 emulator: cold start, splash auto-nav, signup back handling, and org deep link all work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(navigation): cover deep link parsing, route registry, event execution - OrgDeepLinkTest (10 cases): parseStartRoute over valid/invalid scheme, host, path, query params, and URL-encoding (Robolectric for android.net.Uri) - RouteRegistryTest (8 cases): no-arg resolution to NavKey, arg-route null, alias normalization, isValidRoute - SettingsViewModelTest: two cases now execute the NavigationEvent lambda against a FakeNavigator and assert the resulting back stack, covering the event -> Navigator wiring and the new popUpToRoot() logout path Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(navigation): scope nav throttle per-screen instead of globally The 300 ms navigation throttle lived on a single lastMutationTime field on the one Navigator per NavDisplay, making it a global lock across every screen and button: navigating on one screen dropped any navigation from another for 300 ms, so a user could not tap "next" through a flow quickly. Bind each screen its own origin-scoped Navigator (via the per-entry decorator) and gate throttled calls on "is my screen still the top of the stack?" rather than a wall-clock timer: - Navigator gains an `origin` binding and `scopedTo(originContentKey)`; isThrottled() drops a throttled call only when the caller's origin is no longer on top. Unscoped navigators keep the time-based debounce as a fallback for tests and non-entry hosts. - rememberScreenTrackingNavEntryDecorator re-provides LocalNavigator as a view scoped to the entry's contentKey, so every screen gets per-screen double-tap protection with no call-site changes. Result: rapid "next" across different screens all succeed (no time gap), while a double-tap on one screen drops the second navigation (origin no longer top). Also removes the side effect where a non-throttled navigate armed the window. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(navigation): shorten nav fade to 300ms Reduce the NavDisplay transition duration from 500ms to 300ms. On a pop, NavDisplay deliberately keeps the outgoing screen on top for the full fade (targetZIndex = initialZIndex - 1f), so during that window it intercepts taps meant for the revealed screen behind it. Halving the window makes tapping the revealed screen right after Back land reliably, and makes transitions snappier overall. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…enstand#1235) (Greenstand#1303) * docs: add task_22 plan for splitting TreeTrackerDAO (Greenstand#1235) * refactor: extract entity-specific Room DAO interfaces (Greenstand#1235) * refactor: wire new DAOs in AppDatabase and RoomModule (Greenstand#1235) * chore: run codeAnalysis formatting on database DAO files (Greenstand#1235) * docs: add task_24 plan for splitting TreeTrackerDAO (Greenstand#1235) --------- Co-authored-by: Jonathan Muller <elforama@gmail.com>
|
👋 Hi @marsof02, thanks for the contribution! Before this PR can be reviewed and merged, please add:
Just drag-and-drop the file(s) into the PR description and GitHub will upload them. This check re-runs automatically when you edit the description. If this change has no user-visible effect, tick the "no user-visible / UI effect" box in the description (or ask a maintainer to add the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request!
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
./codeAnalysisto automatically apply formatting/linting)Fixes Greenstand#1235 🦕
Summary
PR 6 of 6 for Greenstand#1235 — removes
TreeTrackerDAOand splits tests.Stacks on #4 (branch
refactor/split-treetracker-dao-location-sync), #3, #2, #1, and Greenstand#1303.Merge after previous PRs, or review against that branch only.
Out of scope
The split preserves existing
TreeTrackerDaoTestbehavior. Several update/delete tests use weak assertions that pass without re-querying the DB (e.g. comparing againstUnit, orassertNotEqualson unrelated types). Insert tests that use sharedFakeFileGeneratorfixtures may pass because Room writes generated ids back into those objects.Not introduced by this PR — carried over from the monolith test. Happy to tighten in a follow-up issue/PR.