Skip to content

Program tags#247

Merged
cabarnes merged 7 commits intorovercode:alphafrom
cabarnes:program-tags
Aug 15, 2019
Merged

Program tags#247
cabarnes merged 7 commits intorovercode:alphafrom
cabarnes:program-tags

Conversation

@cabarnes
Copy link
Copy Markdown
Member

Closes #242

Adds owner_tags and admin_tags to programs. Programs can be filtered by a list of tags. The filtering looks at both owner_tags and admin_tags for a match. Right now only owner_tags are set through the API until there is a better definition of who an admin is for setting the admin_tags.

@cabarnes cabarnes requested a review from hbradio August 11, 2019 20:11
@coveralls
Copy link
Copy Markdown

coveralls commented Aug 11, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 66f722e on cabarnes:program-tags into c9109a8 on rovercode:alpha.


@staticmethod
def filter_tags(queryset, _, value):
"""Use all tags when filtering."""
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.

So there is a derived property tags that is the combination of owner_tags and admin_tags, and that's what the query filter searches. Is that right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It can't use the property since that's not available for the database query. But, it does something similar in this method:

Q(owner_tags__name__in=tags) | Q(admin_tags__name__in=tags)

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.

Ah, I see.

I like that, because it would allow me the admin to go set the "lesson 1" admin)tag on a program if the user does not set it as a owner_tag, and user searches will return it.

But, one use case I was thinking of was a section on the landing page for "Featured Programs". I imagined it being populated by querying for admin_tag="featured program". For that, we'd need to be able to query by admin_tag alone, right? Otherwise, a user could set an owner tag of "featured program" and put their program in the section on the landing page.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, for that feature, we'll need to add admin_tags to the BlockDiagramFilter. I'll go ahead and make that change. I can make a filter on owner_tags as well if we think that might be a use case in the future as well

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.

Yeah, probably can't hurt to have every option?

Copy link
Copy Markdown
Collaborator

@hbradio hbradio left a comment

Choose a reason for hiding this comment

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

Works great!

@cabarnes cabarnes merged commit 1549f72 into rovercode:alpha Aug 15, 2019
@cabarnes cabarnes deleted the program-tags branch August 15, 2019 13:21
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.

Add tags to block-diagram model

3 participants