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

Fix submission status #354

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix submission status #354

wants to merge 5 commits into from

Conversation

fritzbrand
Copy link
Contributor

@fritzbrand fritzbrand commented Aug 12, 2024

Purpose

There is a bug on the submissions status fields on standalone templates, where the submissions status fields that show on the listing page, is not always the same as on the edit page.

This is due to revisions, and the listing page only showing published revisions, whereas the edit page shows the latest, regardless of live/draft status,

Solution

To fix this for now, we overwrite the submission status of all revisions, with the information of the last submission attempt

@fritzbrand fritzbrand marked this pull request as ready for review August 20, 2024 05:44
@fritzbrand fritzbrand requested a review from rudigiesler August 20, 2024 05:51
Copy link
Contributor

@rudigiesler rudigiesler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got some concerns around this. We shouldn't be publishing when saving a revision, since that means you can't have anything in draft. We also shouldn't be overwriting revision history, we'll want to be able to see in the history the templates that failed submission.

Also, are there not any unit tests around this? Shouldn't we be adding checks that this submission is updating all the fields that we expect it to?

r.content["submission_name"] = self.template_name
r.content["submission_status"] = self.submission_status
r.content["submission_result"] = self.submission_result
r.save()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be overwriting the whole history of a template whenever we create a submission.

If I'm understanding correctly, the reason the updated status doesn't appear in the list, is that the list only shows the latest published value, not the latest revision. In that case, simply adding the publishing should've fixed this (although publishing on save also doesn't seem correct to me)

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

Successfully merging this pull request may close these issues.

2 participants