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.

Sign up for GitHub

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

Jump to bottom

UniqueWithinRegion doesn't check the base class #16739

Closed
Fryguy opened this issue Jan 3, 2018 · 2 comments
Closed

UniqueWithinRegion doesn't check the base class #16739

Fryguy opened this issue Jan 3, 2018 · 2 comments
Assignees
@d-m-u
Labels
bug core

Comments

@Fryguy
Copy link
Member

Fryguy commented Jan 3, 2018

The UniqueWithinRegion uses record.class, which when used with STI classes would be a leaf class, and may not scope properly across them. It should probably use record.class.base_class instead.

@carbonin Can you work on this when you have a chance?

cc @d-m-u I think you had added a fix using this feature to the CustomizationTemplate class, however that class uses STI, and I don't think the validation works as expected. The PR in question #15495, doesn't seem to have tests that verify this, so can you add specs? We may have just gotten lucky if the only things in the database were PXE customization templates.
@carbonin carbonin assigned d-m-u and unassigned carbonin Jan 3, 2018
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 3, 2018
@d-m-u
Allows us to properly scope for unique_within_region per ManageIQ#16739
9863187
This was referenced Jan 3, 2018
Closed
Merged
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 4, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
2e8e038
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 4, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
b7dc603
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 8, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
61df58f
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 8, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
10abbdb
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 8, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
68a2830
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 8, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
0ef6f8d
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 8, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
32d6c4f
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 9, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
1941c57
@d-m-u d-m-u mentioned this issue Jan 9, 2018
Merged
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 10, 2018
@d-m-u
Adds tests for unique_within_region per ManageIQ#16739
03ef6a1
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 10, 2018
@d-m-u
Fixes unique_within_region and STI per ManageIQ#16739
093b3f5
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 10, 2018
@d-m-u
Fixes unique_within_region and STI per ManageIQ#16739
4d907dd
d-m-u added a commit to d-m-u/manageiq that referenced this issue Jan 10, 2018
@d-m-u
Fixes unique_within_region and STI per ManageIQ#16739
b8402fa
@d-m-u
Copy link
Contributor

d-m-u commented Jan 19, 2018

@Fryguy Can we close this one?

@carbonin
Copy link
Member

Fixed by #16775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@d-m-u d-m-u

Labels
bug core
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Fryguy @carbonin @d-m-u