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

submit report for approval #123

Merged
merged 10 commits into from
Jan 22, 2021
Merged

submit report for approval #123

merged 10 commits into from
Jan 22, 2021

Conversation

jasalisbury
Copy link

@jasalisbury jasalisbury commented Jan 20, 2021

Description of change

A report can now be submitted for approval. Every page must be "completed" before a report can be submitted. A single manager must be assigned as the approving manager.

How to test

  1. Pull down and run migrations
  2. Fill out a complete report
  3. Submit the report, note the review page in navigation is marked "submitted"

Or test it out on sandbox

Issue(s)

Checklist

  • Meets issue criteria
  • Code tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • Documentation updated

@dcmcand
Copy link

dcmcand commented Jan 20, 2021

I am testing this, and it looks like it works, but when I am filling out the form, I get an "unable to save form" error, and the logs show SERVICE:ACTIVITY_REPORTS id: 2 Sequelize error SequelizeDatabaseError: invalid input syntax for type integer: ""

Edit: Investingation shows this was user error, I had forgot to put a grantee name in.

@dcmcand
Copy link

dcmcand commented Jan 20, 2021

It looks like it stays saying submitted even if you go to fill out a new report.

If you skip to submit when submitting a second report, it shows this

Screenshot from 2021-01-20 13-32-10

if you go through the whole second report properly, when you get to the submit stage, it says that it is submitted even before you select an approving manager and press select.

Screenshot from 2021-01-20 13-36-40

ApprovingManagerId was previously being set to "" which postgres didn't
like (since "" is not an integer). Can't simply set "valueAsNumber"
because null values become 0. Explicitly set "0" to null
@jasalisbury
Copy link
Author

I am testing this, and it looks like it works, but when I am filling out the form, I get an "unable to save form" error, and the logs show SERVICE:ACTIVITY_REPORTS id: 2 Sequelize error SequelizeDatabaseError: invalid input syntax for type integer: ""

Edit: Investingation shows this was user error, I had forgot to put a grantee name in.

I think this was an actual bug I just pushed a fix up for. Null approving manager id came across the wire as an empty string ""

@jasalisbury
Copy link
Author

It looks like it stays saying submitted even if you go to fill out a new report.

I think I have the form proper resetting itself, this should be fixed.

@dcmcand
Copy link

dcmcand commented Jan 21, 2021

Still seeing the same issue with submitting twice.

My flow is to create the first complete report and then click on Activity Reports in the nav and start again. I also noticed the second time it saves successfully even without a grantee or non-grantee name.

Screenshot from 2021-01-21 13-23-01

This solves a few problems. Reports are now properly reset when clicking
"new" after submitting a report. Now components can receive the activity
report id if needed (for example file upload).
@jasalisbury
Copy link
Author

Still seeing the same issue with submitting twice.

My flow is to create the first complete report and then click on Activity Reports in the nav and start again. I also noticed the second time it saves successfully even without a grantee or non-grantee name.

Screenshot from 2021-01-21 13-23-01

Alright I think I have it nailed down, but let me know if you still see the same issue!

Copy link

@dcmcand dcmcand left a comment

Choose a reason for hiding this comment

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

Looks good!

@jasalisbury jasalisbury merged commit d72bb39 into main Jan 22, 2021
@jasalisbury jasalisbury deleted the js-135-submit-report branch January 22, 2021 18:59
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.

3 participants