Skip to content

feat: add additional tags support per-channel - #787

Open
whale-food wants to merge 7 commits into
DialmasterOrg:mainfrom
whale-food:main
Open

feat: add additional tags support per-channel#787
whale-food wants to merge 7 commits into
DialmasterOrg:mainfrom
whale-food:main

Conversation

@whale-food

Copy link
Copy Markdown

This will add a new tab to the channel settings dialog that lets you add additional tags separated by | which will automatically add those new tags to the .nfo files that get created when a video downloads.

@dialmaster dialmaster left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry it took me so long to review. Looks pretty good, but I have a few things that I want changed before it can be merged:

  1. Retarget the PR to dev. Contributor PRs go to dev, not main (merging to main triggers a production release). You'll want to base your branch on dev too.
  2. Write the custom tags into the embedded MP4 metadata, not just the NFO. The post-processor already embeds tags as keywords via AtomicParsley (videoDownloadPostProcessFiles.js, the --keyword arg), which is what Plex reads. As written the feature is NFO-only, so it does nothing for Plex users.
  3. Prepend the custom tags instead of appending. The keyword embed caps at 10 tags (slice(0, 10)) and YouTube videos usually have more than that, so appended custom tags would just get cut off.
  4. Make sure rating updates don't wipe the tags. bulkUpdateVideoRatings in videosModule.js regenerates the NFO from the on-disk .info.json, which doesn't know about your custom tags, so changing a video's rating in the UI would silently drop them.
  5. Read additional_tags from settingsChannelRecord instead of channelRecord. Disabled channels aren't supposed to contribute settings (same as default_rating and subfolder).
  6. Add tests: the pipe parsing and tag merging in nfoGenerator.test.js, and the new field in channelSettingsModule.test.js.
  7. Small cleanup: the migration comment and the validateAdditionalTags JSDoc still refer to title_filter_regex / Python regex.

For 2-4, I think the simplest shape is to merge the parsed channel tags into jsonData.tags in the post-processor, before it writes the .info.json back out. Then the NFO writer doesn't need a new parameter, the AtomicParsley embed picks the tags up, and rating regeneration keeps them for free since they're persisted in the .info.json.

LMK if any of that doesn't make sense.

Edit:

  • Make sure you test this on PC and MB. I didn't spin up your branch, but it looks like the mobile styling might cause labels to clip/overlap.
  • User added tags should be deduped so a user can't enter the same tag twice
  • Ensure that invalid characters and whitespace-only tags cannot be added

@whale-food

Copy link
Copy Markdown
Author

That all makes a lot of sense! Thanks so much for reviewing this. That all seems fairly straightforward. I'll try to make these changes soon, but my baby is being born within the week, so I'll likely need some grace on timing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants