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

Clone owner_id to group_id in miq_sets table for dashboards assigned to group #432

Merged

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Nov 13, 2019

This PR should go together with ManageIQ/manageiq#19491

Fixes issue ManageIQ/manageiq#18924

@yrudman yrudman force-pushed the copy_owner_id-to-group_id-for-dashboards branch from 9728104 to d356240 Compare November 13, 2019 19:06
@yrudman yrudman changed the title Clone owner_id to group_id in for dashboards assigned to group Clone owner_id to group_id in miq_sets table for dashboards assigned to group Nov 14, 2019
@yrudman yrudman force-pushed the copy_owner_id-to-group_id-for-dashboards branch from 113b89d to c7bc1b4 Compare November 14, 2019 16:58
@yrudman yrudman force-pushed the copy_owner_id-to-group_id-for-dashboards branch from c7bc1b4 to 927a13d Compare November 14, 2019 17:02
describe CopyOwnerIdToGroupIdForDashboards do
let(:miq_set) { migration_stub(:MiqSet) }
let(:name) { "Hello Dashboard" }
let(:group_id) { 1 }
Copy link
Member

Choose a reason for hiding this comment

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

Don't hardcode the group_id as this is invalid in a multi-region mode. I believe there are specs where we need to create ids from an appropriate region, so search the specs for that...something around ActiveRecord::IdRegions.

Copy link
Member

Choose a reason for hiding this comment

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

As agreed, we'll keep this as is, as it seems we don't have an existing mechanism for generating regionalized ids.

@yrudman yrudman force-pushed the copy_owner_id-to-group_id-for-dashboards branch 4 times, most recently from ae1c430 to ec0715b Compare November 15, 2019 00:06
@miq-bot
Copy link
Member

miq-bot commented Nov 15, 2019

Checked commits yrudman/manageiq-schema@d356240~...ec0715b with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 2 offenses detected

db/migrate/20191112132759_copy_owner_id_to_group_id_for_dashboards.rb

@yrudman yrudman force-pushed the copy_owner_id-to-group_id-for-dashboards branch from ec0715b to 927a13d Compare November 15, 2019 14:48
@Fryguy Fryguy merged commit e881d85 into ManageIQ:master Nov 15, 2019
@Fryguy Fryguy self-assigned this Nov 15, 2019
@Fryguy Fryguy added the data label Nov 15, 2019
@Fryguy Fryguy added this to the Sprint 125 Ending Nov 25, 2019 milestone Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants