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 #20703 - fix host traces page #6938

Merged
merged 1 commit into from Sep 14, 2017

Conversation

Projects
None yet
4 participants
@jlsherrill
Copy link
Member

commented Sep 6, 2017

No description provided.

@theforeman-bot

This comment has been minimized.

Copy link

commented Sep 6, 2017

Issues: #20703

@bbuckingham

This comment has been minimized.

Copy link
Member

commented Sep 7, 2017

@jlsherrill, Are there additional scenarios involving a 'CollectionProxy' where we should apply the same change? Should we cover those with this issue or a separate?

@jlsherrill

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2017

@bbuckingham i don't know of any easy of figuring that out without reading through every controller and hoping that i catch some sort of association being used. I'm open to ideas but I don't have many beyond that.

@bbuckingham

This comment has been minimized.

Copy link
Member

commented Sep 7, 2017

@jlsherrill, Unfortunately, the scenario you describe is likely the only way to determine it (review/inspect/test/fix) on each controller that is using the 'uniq' construct. It was found in another case that 'uniq!' will not convert the CollectionProxy to an Array. That might be the easiest way (s/uniq/uniq!/), but means we could still have 'uniq!' usage in places that it may not be fully needed.

@bbuckingham
Copy link
Member

left a comment

ACK on this PR; however, we should probably have another redmine to address similar issues across the controllers, since we could anticipate similar errors wherever uniq is used on the info passed in to scoped_search().

@Klaas-

Klaas- approved these changes Sep 12, 2017

Copy link
Member

left a comment

I tested the change on a katello 3.4.5 and it works nicely - thanks @jlsherrill

@jlsherrill

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2017

@jlsherrill jlsherrill merged commit 321c007 into Katello:master Sep 14, 2017

3 checks passed

default Job result: SUCCESS
Details
hound No violations found. Woof!
prprocessor Commit message style is correct
Details

@jlsherrill jlsherrill deleted the jlsherrill:20703 branch Sep 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.