Follow-up to #2551727: Menu link migrations should be defensive

Mostly what the title says. We can avoid stubbing and any weirdness involved in that process by relying on the depth field to order dependencies.

CommentFileSizeAuthor
#5 add_orderby_to_menulink-2568331-5-testonly.patch2.48 KBneclimdul
#5 add_orderby_to_menulink-2568331-5.patch5.37 KBneclimdul
#2 add_orderby_to_menulink-2568331-2.patch3.09 KBneclimdul
#2 add_orderby_to_menulink-2568331-2-testonly.patch2.48 KBneclimdul

Comments

neclimdul created an issue. See original summary.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new add_orderby_to_menulink-2568331-2-testonly.patch2.48 KB
new add_orderby_to_menulink-2568331-2.patch3.09 KB

I think the changes in the source db in testonly might cause a failure. at the very least they would cause stubbing. I don't know that its going to be reasonable to test that we don't stub though.

The last submitted patch, 2: add_orderby_to_menulink-2568331-2-testonly.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: add_orderby_to_menulink-2568331-2.patch, failed testing.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new add_orderby_to_menulink-2568331-5.patch5.37 KB
new add_orderby_to_menulink-2568331-5-testonly.patch2.48 KB

Bah, it looks like there where some hardcoded values in the menu tree that didn't match what the ui writes. Just fixed the tests.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: add_orderby_to_menulink-2568331-5-testonly.patch, failed testing.

The last submitted patch, 2: add_orderby_to_menulink-2568331-2-testonly.patch, failed testing.

The last submitted patch, 2: add_orderby_to_menulink-2568331-2.patch, failed testing.

The last submitted patch, 5: add_orderby_to_menulink-2568331-5-testonly.patch, failed testing.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review

Thanks pifr! Sorry for messing up the ordering requiring this comment setting the status back to exist.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Seems legit!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 98be22d on 
    Issue #2568331 by neclimdul: Add orderby to MenuLink sources to avoid...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.