Skip to content

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

Merged
merged 17 commits into from
Dec 6, 2022
Merged

Get previous template version #5230

merged 17 commits into from
Dec 6, 2022

Conversation

BrunoQuaresma
Copy link
Collaborator

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.

@BrunoQuaresma BrunoQuaresma requested a review from a team December 1, 2022 14:36
@BrunoQuaresma BrunoQuaresma self-assigned this Dec 1, 2022
@BrunoQuaresma BrunoQuaresma marked this pull request as ready for review December 2, 2022 13:41
Copy link
Member

@mafredri mafredri left a 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.

BrunoQuaresma and others added 2 commits December 2, 2022 11:32
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Copy link
Member

@mafredri mafredri left a 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).

BrunoQuaresma and others added 2 commits December 5, 2022 13:05
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Copy link
Member

@mafredri mafredri left a 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. 👍🏻

@BrunoQuaresma BrunoQuaresma enabled auto-merge (squash) December 5, 2022 17:43
@BrunoQuaresma BrunoQuaresma merged commit e17fd0b into main Dec 6, 2022
@BrunoQuaresma BrunoQuaresma deleted the bq/previous-version branch December 6, 2022 14:15
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants