Add tag support for announcements

[nworbiii]

Hey, I just made a Pull Request!

Provide finer-grained filtering discovery of Announcements based upon Tags.

Announcements currently define a Category to which they are attributed. However, a category may be too broad. Tags would provide the means to further filter announcements.

Closes #2977

:heavy_check_mark: Checklist

• [x] A changeset describing the change and affected packages. (more info)
• [x] Added or updated documentation
• [x] Tests for new functionality and regression tests for bug fixes
• [x] Screenshots attached (for UI changes)
• [x] All your commits have a Signed-off-by line in the message. (more info)

[backstage-goalie[bot]]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage-community/plugin-announcements-backend	workspaces/announcements/plugins/announcements-backend	minor	v0.5.0
@backstage-community/plugin-announcements-common	workspaces/announcements/plugins/announcements-common	patch	v0.5.0
@backstage-community/plugin-announcements-react	workspaces/announcements/plugins/announcements-react	minor	v0.6.0
@backstage-community/plugin-announcements	workspaces/announcements/plugins/announcements	minor	v0.7.0

[backstage-goalie[bot]]

Thanks for the contribution! All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

[kurtaking]

I'm dealing with an illness while taking care of a sick little one. It'll be a few days before I can review.

[kurtaking]

@gaelgoth - do you have time (and want) to pull this and test it?

@nworbiii - I've reviewed half the code itself. Hoping to have it done early this week as I get back in the swing of things.

[gaelgoth]

Thank you, @nworbiii, for the PR!

I ran a few tests on my end. I noticed that when filling the tags field and pressing Enter, the tag isn't registered in the database, which results in a 500 error when submitting the announcement.

This issue isn't introduced by this feature. I've observed the same behavior with the Category field as well. However, it's less noticeable there since it's a single-entry field.

I find that for the tags field, the feedback that you need to click on “Create ...” to save an entry in the list is less obvious

I'm open to other alternatives, perhaps it would be better to save the array once the user has entered all the tags?

[nworbiii]

Thank you, @nworbiii, for the PR!

I ran a few tests on my end. I noticed that when filling the tags field and pressing Enter, the tag isn't registered in the database, which results in a 500 error when submitting the announcement.

This issue isn't introduced by this feature. I've observed the same behavior with the Category field as well. However, it's less noticeable there since it's a single-entry field.

I find that for the tags field, the feedback that you need to click on “Create ...” to save an entry in the list is less obvious

I'm open to other alternatives, perhaps it would be better to save the array once the user has entered all the tags?

The tags now do not save until the AnnouncementForm is submitted

[nworbiii]

Your branch is currently a bit behind the base branch, could you please update it when you get a chance?

Also, with the recent Backstage upgrade to v1.38, you’ll need to adjust your React component to align with the new JSX transform.

Once that’s done, I’ll do a full review again. Thanks! 👍🏾

Should be good now.

[nworbiii]

• added validation to endpoints
• changed table names from announcements_tags to tags
• used chips array for TagsInput
• added additional handling of duplicates in TagsInput
• fixed TagsContent styling to match CategoriesContent
• fixed regression with tags not displaying in announcements

[nworbiii]

Bump for visibility. PR is ready for review again.

[kurtaking]

How are we feeling about merging once the API report is fixed? @gaelgoth

[nworbiii]

How are we feeling about merging once the API report is fixed? @gaelgoth

Hello. When I run yarn prettier:check locally, it isn't reporting that there are any prettier errors like the CI job is reporting. What am I missing?

[gaelgoth]

Hi @nworbiii, there’s still a bit of refactoring needed to fully support table.json('tags'). For example, we could remove this part into a single implementation by removing if (isPostgres): #3530 (files).

I let you double check that all CRUD are OK with the json format.

To fix formatting, please run yarn prettier:fix.

I’ll focus on your PR so we can ship this feature soon! 🚀

[nworbiii]

Hi @nworbiii, there’s still a bit of refactoring needed to fully support table.json('tags'). For example, we could remove this part into a single implementation by removing if (isPostgres): #3530 (files).

I let you double check that all CRUD are OK with the json format.

To fix formatting, please run yarn prettier:fix.

I’ll focus on your PR so we can ship this feature soon! 🚀

Ah, I'd forgotten to pull the suggested change to my local that was causing the prettier check to fail.

I did the refactor. It seems to be working in both sqlite and pg but please test again.

[gaelgoth]

Ah, it looks like we also need your review on this, @vinzscam

[kurtaking]

Ah, it looks like we also need your review on this, @vinzscam

Reviewing now. If things look good to you and I, I think he will be okay with us dismissing. Their changes were from early on and I'm pretty sure they are addressed.

[kurtaking]

I am failing to create a new announcement with added tags locally. Can someone else test locally with a clean instance using postgres?

edit: should we be calling JSON.stringify on the tags before inserting into the DB? And it might be worth storing all tags as an array, even when there is only one.

[gaelgoth]

Hi @nworbiii, when you have a chance, could you please take a look at @kurtaking feedback?

[nworbiii]

I am failing to create a new announcement with added tags locally. Can someone else test locally with a clean instance using postgres?

edit: should we be calling JSON.stringify on the tags before inserting into the DB? And it might be worth storing all tags as an array, even when there is only one.

Hi @kurtaking. I connected to psql and dropped all of the backstage related databases and then ran db:setup from inside plugin-announcements-backend, saw that everything was wiped, and was able to create a new announcement with a tag. Could you try that? Or if someone else can try to reproduce.

I can make the those extra changes if they are blockers to getting the PR merged.

[gaelgoth]

Hi @nworbiii,

I tested it again on my end, no issues found. Let’s go ahead and ship this feature 🚢