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

Fixes #21484 - Auto complete for Docker Meta Tag #7040

Merged
merged 1 commit into from
Oct 30, 2017

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Oct 27, 2017

This commit adds autocomplete constructs to docker meta tag and removes
the unused ones in docker tag. Idea here is DockerTagsController has now
completely shifted to using the DockerMetaTag model for all data
purposes and hence there is no value in holding auto complete
constructs there.

This commit also includes sensible auto complete values for docker
manifests so that one can search by things like the manifest's digest
name and schema version in the Repository -> Manage Docker Manifests
page.

This commit adds autocomplete constructs to docker meta tag and removes
the unused ones in docker tag. Idea here is DockerTagsController has now
completely shifted to using the DockerMetaTag model for all data
purposes and hence there is no value in holding auto complete
constructs there.

This commit also includes sensible auto complete values for docker
manifests so that one can search by things like the manifest's digest
name and schema version in the Repository -> Manage Docker Manifests
page.
@theforeman-bot
Copy link

Issues: #21484

@parthaa
Copy link
Contributor Author

parthaa commented Oct 27, 2017

To test this PR

  • Sync a Docker Repo
  • Goto Content -> Docker Tags
  • Type anything in the Filters -> you should be able to search by tag, schema_version and repository name
  • Goto Repository Details of the Docker Repo and click on the number next to the DockerManifest list (or tag for that matter)
  • Type anything in the Filters -> you should be able to search by digest and schema_version

@parthaa
Copy link
Contributor Author

parthaa commented Oct 27, 2017

Also note you can search only for '=' in case of schema_version of a DockerMetaTag
screenshot from 2017-10-27 12-22-19

@@ -9,6 +9,8 @@ class DockerManifest < Katello::Model
CONTENT_TYPE = Pulp::DockerManifest::CONTENT_TYPE
scoped_search :on => :name, :complete_value => true
scoped_search :relation => :docker_tags, :on => :name, :rename => :tag, :complete_value => true, :only_explicit => true
scoped_search :on => :digest, :rename => :digest, :complete_value => true, :only_explicit => true
scoped_search :on => :schema_version, :rename => :schema_version, :complete_value => true, :only_explicit => true
Copy link
Member

Choose a reason for hiding this comment

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

Both of these new searches are using only_explicit: true and I point that out because I think you'll have an upcoming PR which removes the name field from DockerManifest which means that all of the scoped search fields are only_explicit: true.

My question is: after removal of the name field, which, if any field should become only_explicit: false and should we make it that way at this time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm good point I can see search by tag name being an automatic thing. That being said I would like to keep it the way it is in this PR because downstream will want what is here.

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense to me, thanks

Copy link
Member

@jturel jturel left a comment

Choose a reason for hiding this comment

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

ACK!

@parthaa parthaa merged commit 6bfcb68 into Katello:master Oct 30, 2017
@parthaa parthaa deleted the auto-complete-meta branch October 30, 2017 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants