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 #10430: Add ability to run a sync plan at any time. #5357

Merged
merged 1 commit into from Jan 26, 2016

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jul 16, 2015

No description provided.

products = item_search(Product, params, options)
respond_for_index(:collection => products)
options = {:includes => [:sync_plan, :provider], :resource_class => Product}
respond_for_index(:collection => scoped_search(products, :name, :desc, options), :template => '../products/index')
Copy link
Member

Choose a reason for hiding this comment

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

errrrr did this get missed in the scoped search conversion?

Copy link
Member

Choose a reason for hiding this comment

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

if thats the case, mind deleting these three lines:

https://github.com/ehelms/katello/blob/fixes-10430/test/controllers/api/v2/sync_plans_controller_test.rb#L31-L33

I know the first one isn't required, i think the 2nd might not anymore either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Appears that it did get missed, I removed those lines you mentioned.

@ehelms
Copy link
Member Author

ehelms commented Aug 21, 2015

Updated to remove the errant $scope in the view.

@ehelms ehelms force-pushed the fixes-10430 branch 4 times, most recently from 18070d5 to 7421922 Compare August 25, 2015 15:39
@ehelms
Copy link
Member Author

ehelms commented Aug 27, 2015

@jlsherrill This is finally ready for re-review after fixing some tests.

@jlsherrill
Copy link
Member

@ehelms works well, one small quirk. If i try to run a sync plan against an a sync plan with no products or a sync plan with only empty products, the server returns an ISE and the UI does not display anything. We probably need to check for that case and display the appropriate error.

@ehelms
Copy link
Member Author

ehelms commented Oct 5, 2015

@jlsherrill updated this based upon ForemanTasks now raising bulk action exceptions

@jlsherrill
Copy link
Member

The current message i get when trying to run an empty sync plan is: 'ERF42-8280 [Foreman::Exception]: Empty bulk action'

wouldn't a more appropriate controller action specific message make more sense?

@ehelms
Copy link
Member Author

ehelms commented Jan 23, 2016

@jlsherrill Updated to include better message

@ehelms ehelms force-pushed the fixes-10430 branch 2 times, most recently from 7b051e8 to e8c2118 Compare January 25, 2016 20:05
@ehelms
Copy link
Member Author

ehelms commented Jan 26, 2016

Tests are now passing

@jlsherrill
Copy link
Member

ACK

ehelms added a commit that referenced this pull request Jan 26, 2016
Fixes #10430: Add ability to run a sync plan at any time.
@ehelms ehelms merged commit 34f720d into Katello:master Jan 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants