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

Adding "approval" concepts to Moves, Orders, and PPMs #508

Merged
merged 5 commits into from
May 14, 2018

Conversation

akostibas
Copy link
Contributor

Description

We need to give the back office the functionality to approve Moves, Orders, and PPMs. I'm taking a stab here for how we might model that in the database and the internal Swagger file.

This RealTimeBoard has a small state diagram illustrating how each record might progress. The circles in Green are currently implemented.

In this PR you'll find:

  • Migrations to add a status field to Orders, Moves, and PPMs.
  • Swagger definitions for those status enums
  • A swagger endpoint for moves/{moveId}/approve to illustrate how we might approve the basics of a move

If we like this approach, we'd probably need to add an approval endpoint for the PPM as well, though this, along with a proper handler, might be sufficient for the upcoming demo.

Since the Back Office UI doesn't explicitly have "orders" approval (only "Basics" and "PPM"), we might set the orders & move statuses to Approved together in the same API call.

screen shot 2018-05-13 at 10 54 09 pm

Code Review Verification Steps

  • All tests pass.
  • Code follows the guidelines for Logging
  • Any migrations/schema changes also update the diagram in docs/schema/dp3.sqs
  • Request review from a member of a different team.
  • (TRIAL) Have the Pivotal acceptance criteria been met for this change?

References

Copy link
Contributor

@tinyels tinyels left a comment

Choose a reason for hiding this comment

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

Could we rename AWAITING_REVIEW to SUBMITTED? It seems we would want to have an endpoint for the SM flow (/moves/{moveid}/submit) that submits the move for review.

The service member currently has a "isProfileComplete" method that just checks to see if there are values in all the right slots. Should we switch this to a status enum too? It's possible that the office might need corrections for some of these items (e.g. backup address is a variation on the residential address)

Copy link
Contributor

@jim jim left a comment

Choose a reason for hiding this comment

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

Code LGTM.

  • I agree with @tinyels re: renaming AWAITING_REVIEW to SUBMITTED.
  • I'm also curious when a move is marked as complete. Is that a manual action by an office user that closes out a move?

@akostibas akostibas changed the title WIP: Adding "approval" concepts to Moves, Orders, and PPMs Adding "approval" concepts to Moves, Orders, and PPMs May 14, 2018
@@ -0,0 +1,3 @@
add_column("moves", "status", "string", {"default": "draft"})
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these defaults need to be uppercase to match, e.g. DRAFT

Copy link
Contributor

@stangah stangah left a comment

Choose a reason for hiding this comment

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

lgtm

@akostibas
Copy link
Contributor Author

@tinyels
👍 "awaiting_review" -> "submitted"

As for isProfileComplete... I don't know. There are a lot of state combinations, and I'm tempted to say we keep some of the complexity down by keeping things inside as few statuses as possible.

The isProfileComplete check could be useful when looking to see if we can make a state transition from DRAFT -> SUBMITTED.

And yes, we will have a troubleshooting flow where the office can kick a move back to the SM to fix / update fields. I don't really know what that will look like yet, but it's alluded to in the "troubleshooting" state in the RealTimeBoard.

@akostibas
Copy link
Contributor Author

@jim

I'm also curious when a move is marked as complete. Is that a manual action by an office user that closes out a move?

There will be some close out flow, which may or may not make use of the COMPLETE status (we still need to design that.) As written, it's sort of a placeholder for showing that a move can eventually be finished-and-done-with, thus no longer needing to be looked at.

Copy link
Contributor

@tinyels tinyels left a comment

Choose a reason for hiding this comment

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

LGTM

@akostibas akostibas merged commit f9cefee into master May 14, 2018
@stangah stangah deleted the ak-move-status-db branch May 14, 2018 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants