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 #36269 - Add check_needs_publish param and logic to publish API #10523

Merged
merged 1 commit into from Apr 24, 2023

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Apr 18, 2023

What are the changes introduced in this pull request?

Adds a check_needs_publish parameter to the content view publish API (That defaults to false) and determine if publish is needed on CV and don't publish if not needed.

Considerations taken when implementing this change?

The needs_publish method is currently implemented for repository change auditing. There's a PR out for factoring in filter + filter rules changes and this PR should work out of the box when merged.

What are the testing steps for this pull request?

  1. Create a CV.
  2. Publish.
  3. In hammer, run hammer content-view publish --id 2 --check-needs-publish true
  4. You should see the message below:
Could not publish the content view:
  Content view does not need a publish since there are no audited changes since the last publish. Pass check_needs_publish parameter as false if you don't want to check if content view needs a publish.

  1. In hammer, run hammer content-view publish --id 2 --check-needs-publish false or In hammer, run hammer content-view publish --id 2`
  2. The content-view should get published regardless of needs_publish value.

@theforeman-bot
Copy link

Issues: #36269

@@ -109,23 +109,14 @@ def update
param :major, :number, :desc => N_("Override the major version number"), :required => false
param :minor, :number, :desc => N_("Override the minor version number"), :required => false
param :environment_ids, Array, :desc => N_("Identifiers for Lifecycle Environment"), :required => false
param :is_force_promote, :bool, :desc => N_("force content view promotion and bypass lifecycle environment restriction"), :required => false
param :check_needs_publish, :bool, :desc => N_("Check if content view needs a publish even based on audited changes since last publish"), :required => false
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
param :check_needs_publish, :bool, :desc => N_("Check if content view needs a publish even based on audited changes since last publish"), :required => false
param :publish_if_needed, :bool, :desc => N_("Check if content view needs a publish even based on audited changes since last publish"), :required => false

I dont know if this sounds better. @jeremylenz what da ya think

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'd go even farther

Suggested change
param :check_needs_publish, :bool, :desc => N_("Check if content view needs a publish even based on audited changes since last publish"), :required => false
param :publish_only_if_needed, :bool, :desc => N_("Check audited changes and proceed only if content or filters have changed since last publish"), :required => false

@sjha4 sjha4 merged commit e75e035 into Katello:master Apr 24, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants