-
Notifications
You must be signed in to change notification settings - Fork 888
Get previous template version #5230
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
Conversation
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.
Didn't fully understand the logic in the fake db but maybe I've misunderstood what we're trying to do there, also had some questions about the query. In general code looks good.
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
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.
Ok, after your explanations Bruno and looking at this again, query and database fake implementation looks fine (except fake was missing one thing, see suggestion).
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
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.
It'd be nice if you could do the remaining errors
=> xerrors
and error block changes, but otherwise looks great. 👍🏻
We need to get the previous template version in the UI, mostly to get the files related to it and show the diff between them on the template version page. so I'm going to add an endpoint for that. In the meantime, I would appreciate your thoughts about the incoming changes like the SQL query and database fake logic.