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

feat: pre-fill existing tags for insights, actions, event definitions, and property definitions #12544

Merged
merged 12 commits into from
Nov 24, 2022

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Nov 1, 2022

Problem

closes #10678

There are several pages where you can add tags to an item. But we only suggest existing tags on one of them.

Changes

Suggest tags on insight, action, event definitions, and property definition pages

Adds an API to event definitions and property definitions to allow this. There's no pagination but there are (at least for now) very few tags on these types.

property definitions

property_definitions

### event definitions

event_definition

insights

insights-tags

actions

actions-tags

How did you test this code?

added developer tests and checked in the UI locally

@pauldambra pauldambra requested a review from Twixes November 1, 2022 09:17
Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Hmm.. I think tags should be global. That means, if you add a tag "marketing" for an insight, the same tag should be available when you tag actions. There's a lot of code duplication here to prevent that from being a thing.

Also, to avoid refactoring this later, I think we should use the taxonomic filter to select tags, giving us autocompletion and pagination virtually for free.

@pauldambra
Copy link
Member Author

I think tags should be global.

fair

we should use the taxonomic filter

Oh, I hate changing the taxonomic filter 🤣 . I think I'll split this into two steps ✂️

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@pauldambra
Copy link
Member Author

Tags are now project "global"

tags

Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

I could nit on a thing here or there, but I won't 😅. Looks good to me! 👍

@mariusandra mariusandra merged commit 0013919 into master Nov 24, 2022
@mariusandra mariusandra deleted the feat/pre-fill-tags branch November 24, 2022 23:42
@pauldambra pauldambra mentioned this pull request Jan 10, 2023
4 tasks
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.

Pre-populate existing tags
3 participants