Feature/[smd 151] show error state on errors screen

[Connor-OS]

This PR

Refactor so we no longer return un-necesary success boolian.

When a user goes to upload a file they will now only see .xlsx files that are on there machine instead of all types of files

Change description

A brief description of the pull request

• Unit tests and other appropriate tests added or updated
• README and other documentation has been updated / added (if needed)
• Commit messages are meaningful and follow good commit message guidelines (e.g. "FS-XXXX: Add margin to nav items preventing overlapping of logo")

How to test

If manual testing is needed, give suggested testing steps

Screenshots of UI changes (if applicable)

[Joel-Pearce]

Do we ideally want some testing on this? There's a lot of conditional code here, and it currently has no test coverage.

It's possible to test JS EventListeners, but how we integrate that with our Flask test suite is another question.

[Connor-OS]

Do we ideally want some testing on this? There's a lot of conditional code here, and it currently has no test coverage.

It's possible to test JS EventListeners, but how we integrate that with our Flask test suite is another question.

Yes, I think we definatly do. I suppose I can do some "black box" type of testing with Beautiful soup to check the on page HTML is what we expect in certain cases.

[Joel-Pearce]

Do we ideally want some testing on this? There's a lot of conditional code here, and it currently has no test coverage.
It's possible to test JS EventListeners, but how we integrate that with our Flask test suite is another question.

Yes, I think we definatly do. I suppose I can do some "black box" type of testing with Beautiful soup to check the on page HTML is what we expect in certain cases.

We can add in some tests asserting correct output against a series of reasonable scenarios. Probably best to get Adam's input on unit testing the specific event listener - it will require us to update github actions, and presumably to run a second JS test suite alongside the current Python one.

[Joel-Pearce]

LGTM

[cyrusdobbs]

requesting changes to block merge for now

[cyrusdobbs]

Looking better 👍

I'm still a bit worried that this still doesn't solve the scenario where:

1. User submits for a place
2. gets validation errors
3. submits again after fixing some, but now gets a pre-validation error
4. It will now show their old validation errors AND their new pre-validation error

We need to be clear with design that this is a side effect of the new behaviour.

[Connor-OS]

Looking better 👍

I'm still a bit worried that this still doesn't solve the scenario where:

1. User submits for a place

2. gets validation errors

3. submits again after fixing some, but now gets a pre-validation error

4. It will now show their old validation errors AND their new pre-validation error

We need to be clear with design that this is a side effect of the new behaviour.

They will now only get "same screen pre-validation" errors if they have a file error. From what design have said they do not consider these "same screen" errors as being another whole upload attempt/ moving to a new screen so the content in the table shouldn't be updated. I belive that design have accepted this iteration currently in the PR

[cyrusdobbs]

LGTM