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

Remove orphaned v2v migration shortcuts #21625

Merged

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Dec 16, 2021

Fixes ManageIQ/manageiq-ui-classic#7994

Ironically, they were neglected when we added v2v and added later in commit 53e764f
Of course, I missed them when removing v2v in:
#21515

Which was part of the larger issue:
#21379

NOTE: v2v removal wasn't in morphy, so this is morphy/no

Fixes ManageIQ/manageiq-ui-classic#7994

Ironically, they were neglected when we added v2v and added later in commit 53e764f
Of course, I missed them when removing v2v in:
ManageIQ#21515

Which was part of the larger issue:
ManageIQ#21379
@agrare
Copy link
Member

agrare commented Dec 16, 2021

@jrafanie we didn't backport the v2v removal right? I still see manageiq-v2v in the upstream/morphy Gemfile

@jrafanie
Copy link
Member Author

jrafanie commented Dec 16, 2021

@jrafanie we didn't backport the v2v removal right? I still see manageiq-v2v in the upstream/morphy Gemfile

Correct, v2v is in morphy so no need to backport this. I'll put the morphy/no label just to ensure no one else checks.

@miq-bot
Copy link
Member

miq-bot commented Dec 16, 2021

Checked commit jrafanie@f688fa0 with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@agrare agrare self-assigned this Dec 16, 2021
@agrare agrare merged commit 1c281f0 into ManageIQ:master Dec 17, 2021
@jrafanie jrafanie deleted the remove_orphaned_v2v_migration_shortcuts branch December 17, 2021 14:07
@Fryguy
Copy link
Member

Fryguy commented Dec 20, 2021

@jrafanie Technically this needs a data migration as well, if anyone actually chose this as their default shortcut in their user preferences.

@jrafanie
Copy link
Member Author

@jrafanie Technically this needs a data migration as well, if anyone actually chose this as their default shortcut in their user preferences.

Interesting. Where is this defined in the UI? I'll have to take a look. Note, MiqShortcut.seed will remove shortcuts as part of db:seed but I can test to ensure it does the right thing when your preferences contain an orphaned shortcut.

@jrafanie
Copy link
Member Author

jrafanie commented Dec 20, 2021

For future me, this is where you find MiqShortcuts...

image

EDIT: Better image showing the old shortcuts that I'm trying to remove.

I guess they're really dashboard widget menu links. 😆

Now to see what happens when I save them and let seed orphan them by deleting the MiqShortcut.

@jrafanie
Copy link
Member Author

Ok, so it was busted when I didn't remove v2v properly:

image

image

@jrafanie
Copy link
Member Author

jrafanie commented Dec 20, 2021

It looks like it works fine NOW after this PR because MiqShortcut has dependent destroy on MiqWidgetShortcut.

Before:

### The 3 shortcuts deleted in this PR:
irb(main):025:0> pp  MiqShortcut.where("url ILIKE '%migration%'")
[#<MiqShortcut:0x00007fe0a76b1018
  id: 203,
  name: "migration",
  description: "Migration / Migration Plans",
  url: "/migration/#plans",
  rbac_feature_name: "migration",
  startup: true,
  sequence: 58>,
 #<MiqShortcut:0x00007fe0a76b0f50
  id: 204,
  name: "mappings",
  description: "Migration / Infrastructure Mappings",
  url: "/migration/#mappings",
  rbac_feature_name: "mappings",
  startup: true,
  sequence: 59>,
 #<MiqShortcut:0x00007fe0a76b0e88
  id: 205,
  name: "migration_settings",
  description: "Migration / Migration Settings",
  url: "/migration/#settings",
  rbac_feature_name: "migration_settings",
  startup: true,
  sequence: 60>]


### The has_many through table MiqWidgetShortcut linking shortcuts to widgets:
irb(main):029:0> pp MiqWidgetShortcut.all
[#<MiqWidgetShortcut:0x00007fe0f2e13d60
  id: 1,
  description: "Migration / Migration Plans",
  miq_shortcut_id: 203,
  miq_widget_id: 83,
  sequence: 0>,
 #<MiqWidgetShortcut:0x00007fe0f2e13c98
  id: 2,
  description: "Migration / Infrastructure Mappings",
  miq_shortcut_id: 204,
  miq_widget_id: 83,
  sequence: 1>,
 #<MiqWidgetShortcut:0x00007fe0f2e13bd0
  id: 3,
  description: "Migration / Migration Settings",
  miq_shortcut_id: 205,
  miq_widget_id: 83,
  sequence: 2>,
 #<MiqWidgetShortcut:0x00007fe0f2e13b08
  id: 4,
  description: "Overview / Reports",
  miq_shortcut_id: 2,
  miq_widget_id: 83,
  sequence: 3>]

### Note, 3 are the migration (v2v) ones and 1 is unrelated, reports.

After seeding on master:

irb(main):001:0> pp  MiqShortcut.where("url ILIKE '%migration%'")
[]

### v2v migration shortcuts are gone, the reports widget shorcut remains:
irb(main):002:0> pp MiqWidgetShortcut.all
[#<MiqWidgetShortcut:0x00007feb11bfe6a0
  id: 4,
  description: "Overview / Reports",
  miq_shortcut_id: 2,
  miq_widget_id: 83,
  sequence: 3>]

UI also looks fine now:

image

The "links" shows only the 1 selected miq shortcut, for overview reports

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when logging in
4 participants