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

[Chapters] Parse toc and modify index #2807

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

danielebogo
Copy link
Contributor

@danielebogo danielebogo commented Mar 6, 2025

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.

To test

  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

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@danielebogo danielebogo added this to the 7.84 ❄️ milestone Mar 6, 2025
@danielebogo danielebogo marked this pull request as ready for review March 6, 2025 15:11
@danielebogo danielebogo requested a review from a team as a code owner March 6, 2025 15:11
Copy link
Contributor

@SergioEstevao SergioEstevao left a comment

Choose a reason for hiding this comment

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

Followed the PR test steps and all is working correctly:

  • Only toc chapters were visible
  • None was deselected because toc hidden chapters were deselected on the original branch

:shipit:

@danielebogo danielebogo changed the base branch from release/7.84 to trunk March 11, 2025 19:18
# Conflicts:
#	Modules/Utils/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift
@danielebogo danielebogo modified the milestones: 7.84 ❄️, 7.85 Mar 11, 2025
Copy link
Member

@leandroalonso leandroalonso left a comment

Choose a reason for hiding this comment

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

@danielebogo chapters are not skipping as they should. Here's the test I did:

  1. Went on develop and deselected some chapters
  2. Ran this branch/app
  3. Tap play
  4. Once the chapter "Fire by climate" finishes I expected "New uniform" to play. It didn't, it just ket playing, as you can see below:
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-03-11.at.16.44.53.mp4

@pocketcasts pocketcasts modified the milestones: 7.85, 7.86 Mar 17, 2025
@pocketcasts
Copy link
Contributor

Version 7.85 has now entered code-freeze, so the milestone of this PR has been updated to 7.86.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants