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 #37140 - Update web UI for SCA-only #10880

Merged
merged 4 commits into from
Feb 14, 2024

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Feb 2, 2024

What are the changes introduced in this pull request?

  1. Update the Organization list page to remove the 'Simple Content Access' column
  2. Update the Organization Create and Edit pages to remove the SCA checkbox and banner
  3. Update Katello::OrganizationCreator to no longer accept an sca: keyword on initialization.
  4. Remove the non-SCA codepaths from RepositorySets Tab on the new host details UI
  5. Remove the SCA toggle React components from the SubscriptionsPage, which was already dead code anyway

Considerations taken when implementing this change?

I'm trying to keep this PR to only removing visual elements that users would see in the web UI, rather than API endpoints or other backend code. But I did have to touch a few places that weren't strictly UI code.

I'm not bothering to surgically remove things from the legacy content host Angular UI, since that entire UI is going away soon anyway, and is already not displayed to users since all organizations now use SCA.

What are the testing steps for this pull request?

Check all the locations above and confirm that UI elements are removed
Poke around and make sure nothing is broken
Make sure CI tests pass and make sense

@jeremylenz jeremylenz force-pushed the 37140-sca-never-heard-of-it branch 2 times, most recently from 3fcda8d to 1476a39 Compare February 5, 2024 15:07
@jeremylenz jeremylenz marked this pull request as ready for review February 5, 2024 20:14
@jeremylenz
Copy link
Member Author

Many test failures should be fixed when #10861 is merged.

@chris1984
Copy link
Member

@jeremylenz Do you want me to make a pr to pull stuff out of the old UI or say just leave like you said since it's going away.

@jeremylenz
Copy link
Member Author

Do you want me to make a pr to pull stuff out of the old UI or say just leave like you said since it's going away.

The cleanup is in a future epic; I think it will be fine until then

@chris1984 chris1984 self-assigned this Feb 6, 2024
@chris1984
Copy link
Member

So far it looks good, still checking a few pages and hammer for the api param removal, then will look at the code. I found this column on the host page, should we remove Subscription Status?

hostsub

@jeremylenz
Copy link
Member Author

I found this column on the host page, should we remove Subscription Status?

That's covered in #10882 :)

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

Works fine with hammer and checked the UI and all of the pages you touched are updated correctly and poked around on some other pages and found an issue on the all hosts page - see comment below.

Update the Organization list page to remove the 'Simple Content Access' column - Pass
Update the Organization Create and Edit pages to remove the SCA checkbox and banner - Pass
Update Katello::OrganizationCreator to no longer accept an sca: keyword on initialization - Pass

As we discussed I found this with your other pr when testing this one on the all hosts page:

hostsub

dbissue

Which you will fix in your other PR. Thanks for getting this fixed up and when we get to the other epic for the old UI, I will get a PR for that going. Code looks fine too from a Ruby/JS standpoint.

@chris1984
Copy link
Member

chris1984 commented Feb 7, 2024

Tested with #10882 and I do not hit the DB error anymore and the Subscription Status column is gone after the migration went through.

@jeremylenz
Copy link
Member Author

[test katello]

@jeremylenz jeremylenz force-pushed the 37140-sca-never-heard-of-it branch from ae271a9 to 36d1bf6 Compare February 14, 2024 14:50
@jeremylenz
Copy link
Member Author

[test katello]

@jeremylenz jeremylenz force-pushed the 37140-sca-never-heard-of-it branch from 36d1bf6 to 500f225 Compare February 14, 2024 20:17
@jeremylenz jeremylenz merged commit 1bd84c2 into Katello:master Feb 14, 2024
15 of 16 checks passed
adamlazik1 added a commit to adamlazik1/katello that referenced this pull request Nov 20, 2024
The label field was removed probably accidentally in Katello#10880.
This action is still doable in api so it should be doable in UI as well.
chris1984 pushed a commit that referenced this pull request Dec 5, 2024
The label field was removed probably accidentally in #10880.
This action is still doable in api so it should be doable in UI as well.
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.

2 participants