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

Refactor bookmark delete deep linking #2453

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Conversation

MiSikora
Copy link
Contributor

@MiSikora MiSikora commented Jul 5, 2024

Description

Note

This will be a series of PRs that move deep linking to a separate module that can be tested. The goal is to address #2405 but I don't want to do it blindly without having tests in order to not introduce regressions to deep linking.

This PR migrates deleting bookmark deep linking to the new module.

Testing Instructions

You can test it by assigning Add bookmark action to your headphones, and then triggering it while the app is in background and choosing Delete option. If you can't test it this way do the following:

  1. Create a bookmark.
  2. Using app manger find an UUID of a bookmark.
  3. Execute adb shell am start -n au.com.shiftyjelly.pocketcasts.debug/au.com.shiftyjelly.pocketcasts.ui.MainActivity -a INTENT_OPEN_APP_DELETE_BOOKMARK -e bookmark_uuid <BOOKMARK_UUID>.
  4. The bookmark should be deleted.

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@MiSikora MiSikora added this to the 7.69 milestone Jul 5, 2024
@MiSikora MiSikora requested a review from a team as a code owner July 5, 2024 10:05
@MiSikora MiSikora requested review from mebarbosa and removed request for a team July 5, 2024 10:05
@dangermattic
Copy link
Collaborator

dangermattic commented Jul 5, 2024

1 Warning
⚠️ Class DeleteBookmarkAdapter is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by 🚫 Danger

@mebarbosa mebarbosa requested review from a team and mebarbosa and removed request for mebarbosa and a team July 5, 2024 14:30
@mebarbosa
Copy link
Contributor

I am removing myself from this PR review since I don't have headphones to test it

@Automattic/pocket-casts-android

@mebarbosa mebarbosa requested review from geekygecko and ashiagr and removed request for mebarbosa July 5, 2024 14:31
Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for migrating all bookmark deep links to DeepLink and adding tests!

@MiSikora MiSikora force-pushed the task/bookmark-show-deep-linking branch from e153599 to b0ee92a Compare July 8, 2024 20:53
Base automatically changed from task/bookmark-show-deep-linking to main July 8, 2024 21:15
@MiSikora MiSikora force-pushed the task/bookmark-delete-deep-link branch from 24f30cd to 5a222e0 Compare July 8, 2024 21:16
@MiSikora MiSikora enabled auto-merge (squash) July 8, 2024 21:23
@MiSikora MiSikora merged commit ebfc3e6 into main Jul 8, 2024
14 checks passed
@MiSikora MiSikora deleted the task/bookmark-delete-deep-link branch July 8, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants