-
Notifications
You must be signed in to change notification settings - Fork 1
Add recording tag model #264
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
Conversation
BryonLewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This completely works functionally given the requirements of issue. The only 'real' requested change is some minor typing stuff in the client.
I have some questions about the functionality that may require some group discussion. Mostly these are questions that if they change the requirements I think would be better addressed in this PR instead of trying to do migration in the future (mostly the idea of multiple tags being associated with a recoring and if we want to support that). I just bring up the multiple tags because my experience indicates that someone is going to ask for it down the line.
0c60f19 to
156e19d
Compare
3756ebe to
304c900
Compare
304c900 to
f315b0c
Compare
|
Change |
bats_ai/core/migrations/0024_remove_recording_tag_recording_tags.py
Outdated
Show resolved
Hide resolved
BryonLewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the multi tag stuff
Just a few minor things:
The patch request on Recording and modification so it accepts multiple new tags and allows for removal of tags
See if the number of migration files can be reduced by recreating the migrations
The idea of uniqueness and tags being tied to a user can be visited in the future. Don't do anything but just think about it a bit.
3d70e41 to
2a36084
Compare
Co-authored-by: Bryon Lewis <61746913+BryonLewis@users.noreply.github.com>
2a36084 to
8159e92
Compare
BryonLewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Fix #259
Adds the ability to add a tag to recordings on upload or update. Displays the tag in the recordings table, and allows for filtering by one or more tags.
Here is what model management looks like for tags:

Each recording tag has a text value and belongs to a specific user. When that user is logged in, they can choose from pre-existing tags that they've created, or type in a new tag:

Each table also has a drop-down on top to enable filtering by tag. The tag column is sortable.

Todo: