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

Filter out non-toc chapters #3556

Merged
merged 3 commits into from
Feb 7, 2025
Merged

Filter out non-toc chapters #3556

merged 3 commits into from
Feb 7, 2025

Conversation

MiSikora
Copy link
Contributor

@MiSikora MiSikora commented Feb 6, 2025

Description

This PR introduces filtering out chapters that should not be in the table of contents. See the specification for more details.

The implementation relies on splitting real index and UI index. Real index is stored and is always used for syncing pre-selected chapters. While UI index is computed on-the-fly after we filter out all invalid chapters, etc.

In the database migration I had to delete all cached remote chapters as we don't have information whether such chapters should be displayed or not.

Fixes #3480

Testing Instructions

  1. Install the app from the main branch.
  2. Subscribe to the No Agenda podcast.
  3. Play the 1728 - "Hatchet Man" - No Agenda episode.
  4. See chapters.
  5. Deselect a chapter like X sats from Y.
  6. Close the app.
  7. Install the app from task/chaptes-toc branch.
  8. Go back to the chapters from the 4th step.
  9. Verify that you don't see X sats from Y chapters and no chapters got de-selected.

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 do not merge [Type] Enhancement Improve an existing feature. [Area] Chapters Episode chapters labels Feb 6, 2025
@MiSikora MiSikora added this to the 7.83 milestone Feb 6, 2025
@MiSikora MiSikora requested a review from a team as a code owner February 6, 2025 09:47
@MiSikora MiSikora requested review from geekygecko and removed request for a team February 6, 2025 09:47
@dangermattic
Copy link
Collaborator

dangermattic commented Feb 6, 2025

1 Warning
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 6, 2025

📲 You can test the changes from this Pull Request in 📱 Mobile by scanning the QR code below to install the corresponding build.
App Name 📱 Mobile
Build TypedebugProd
Commit1933f30
Direct Downloadpocketcasts-app-prototype-build-pr3556-1933f30.apk
📲 You can test the changes from this Pull Request in 🚗 Automotive by scanning the QR code below to install the corresponding build.
App Name 🚗 Automotive
Build TypedebugProd
Commit1933f30
Direct Downloadpocketcasts-automotive-prototype-build-pr3556-1933f30.apk
📲 You can test the changes from this Pull Request in ⌚ Wear by scanning the QR code below to install the corresponding build.
App Name ⌚ Wear
Build TypedebugProd
Commit1933f30
Direct Downloadpocketcasts-wear-prototype-build-pr3556-1933f30.apk

Base automatically changed from task/chapters-cleanup to main February 6, 2025 13:12
Copy link
Member

@geekygecko geekygecko left a comment

Choose a reason for hiding this comment

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

This worked well in my test. Thanks for making this fix!

@geekygecko geekygecko merged commit ffedc45 into main Feb 7, 2025
17 checks passed
@geekygecko geekygecko deleted the task/chapters-toc branch February 7, 2025 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Area] Chapters Episode chapters [Type] Enhancement Improve an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the Podcast Index chapters attribute 'toc'
4 participants