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 #33720 - Move collections to generic UI #9731

Merged
merged 1 commit into from Oct 28, 2021

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Oct 18, 2021

What are the changes introduced in this pull request?

Move ansible collections to the new generic unit page.

  1. /ansible_collections and Content > Ansible Collections from nav menu now lands on the generic content UI
  2. legacy_ansible_collections will hold the PF3 pages for a release before the code is deleted.

What are the testing steps for this pull request?

  1. Go to nav menu > Content > Ansible Collections
  2. Go to Nav menu > content > Other content types: The content type selector menu should have ansible_collections as an option.

To Do:

  • Tests

@theforeman-bot
Copy link

Issues: #33720

Copy link
Member

@rverdile rverdile left a comment

Choose a reason for hiding this comment

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

I think content_type in GenericContentType can be removed since it's being initialized in ContentType

class GenericContentType < ContentType
attr_accessor :pulp3_api, :pulp3_model, :content_type, :filename_key, :duplicates_allowed, :pluralized_name,
:model_name, :model_version, :model_filename
def initialize(options)
super
self.pulp3_api = options[:pulp3_api]
self.pulp3_model = options[:pulp3_model]
self.content_type = options[:content_type]

Copy link
Member

@rverdile rverdile left a comment

Choose a reason for hiding this comment

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

Should the old UI url be legacy/ansible_collections to be consistent with content views, legacy/content_views?

Copy link
Member

@rverdile rverdile left a comment

Choose a reason for hiding this comment

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

I thought I fixed this in my PR, but it looks like the repositories tab for content view details is still broken. I'm gonna look into why that is.

Other than that, everything looks good! Not finding any other issues.

@rverdile
Copy link
Member

I thought I fixed this in my PR, but it looks like the repositories tab for content view details is still broken. I'm gonna look into why that is.

This was caused by ostree, so it's nothing to worry about here.

@sjha4 sjha4 force-pushed the ansible_collection_generic_ui branch from 0c717cd to c872a12 Compare October 26, 2021 14:02
@sjha4
Copy link
Member Author

sjha4 commented Oct 26, 2021

[test katello]

@theforeman-bot
Copy link

@sjha4, this pull request is currently not mergeable. Please rebase against the master branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream master

This message was auto-generated by Foreman's prprocessor

{
title: __('Product'),
getProperty: unit =>
<a href={urlBuilder(`products/${unit?.product.id}/`, '')}>{unit?.product.name}</a>,
Copy link
Member

Choose a reason for hiding this comment

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

very small thing, but there's an extra slash on the end here. products/${unit?.product.id}/ <----

Copy link
Member

@rverdile rverdile left a comment

Choose a reason for hiding this comment

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

Looks good! There's that one slash I pointed out, but that doesn't really break anything.

@sjha4 sjha4 force-pushed the ansible_collection_generic_ui branch from fd19776 to 33545ca Compare October 27, 2021 21:10
@sjha4 sjha4 merged commit 8c6803a into Katello:master Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants