-
Notifications
You must be signed in to change notification settings - Fork 12.2k
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
LibraryPanels: Don't include ScopedVars with persisted model #67843
Conversation
@dprokop suggested adding a release note if we're potentially breaking the behavior of the app. Double checking if this is a real breaking change. |
I played around with both cases: the bug and the solution. It seems to me that we'd be introducing a bug in place of another bug. Now instead of having this problem: Screen.Recording.2023-05-11.at.10.20.01.movWe'd have this problem: Screen.Recording.2023-05-11.at.10.17.08.movI don't think it's that much better. |
@polibb How about with the last commit? |
@@ -64,7 +64,7 @@ export async function getLibraryPanel(uid: string, isHandled = false): Promise<L | |||
schemaVersion: 35, // should be saved in the library panel | |||
panels: [result.model], | |||
}); | |||
const model = dash.panels[0].getSaveModel(); // migrated panel | |||
const { scopedVars, ...model } = dash.panels[0].getSaveModel(); // migrated panel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getSaveModel should never include scopedVars, it should be safe to add scopedVars to non persisted props in PanelModel?
It is cleaned as part of DashboardModel.getPanelSaveModels but maybe it's better to do that in PanelModel ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works as expected! 😊 🙌
not going over @torkelo's comment, but approving the manual testing and additions on loadLibraryPanelAndUpdate
Removes ScopedVars from library panel before saving.
Unfortunately this highlights another issue in that repeats are processed before library panels are resolved, so if a library panel is repeated by a variable, that variable's value needs to be refreshed or changed for repeats to be processed.
I think it's possible this is a regression introduced during the library panel frontend move, but I'd have to investigate.
Closes #65518
Note: I've marked this as a breaking change since I guess it's possible someone somewhere depended on this behavior.