Merge pull request #1743 from 18F/wml-3663-kickback-status

[3663] set kickback status

api/account.go

@@ -29,6 +29,15 @@ var knownFormVersions = map[string][]string{
 	},
 }
 
+const (
+	// StatusIncomplete indicates that the submission is INCOMPLETE
+	StatusIncomplete = "INCOMPLETE"
+	// StatusSubmitted indicates that the submission is SUBMITTED
+	StatusSubmitted = "SUBMITTED"
+	// StatusKickback indicates that the submission is KICKBACK
+	StatusKickback = "KICKBACK"
+)
+
 // Account represents a user account
 type Account struct {
 	ID          int
@@ -38,7 +47,7 @@ type Account struct {
 	Token       string
 	TokenUsed   bool
 	Email       string
-	Locked      bool
+	Status      string
 	FormType    string `db:"form_type"`
 	FormVersion string `db:"form_version"`
 	ExternalID  string `db:"external_id"` // ExternalID is an identifier used by external systems to id applications
@@ -137,18 +146,45 @@ func (entity *Account) FindByExternalID(context DatabaseService) error {
 	return context.Where(entity, "Account.external_id = ?", entity.ExternalID)
 }
 
-// Lock will mark the account in a `locked` status.
-func (entity *Account) Lock(context DatabaseService) error {
-	entity.Locked = true
-	_, err := entity.Save(context, entity.ID)
-	return err
+// CanSubmit returns wether the account is in a valid state for submission
+func (entity *Account) CanSubmit() bool {
+	if entity.Status == StatusSubmitted {
+		return false
+	}
+	return true
+}
+
+// Submit will mark the account in a `SUBMITTED` status.
+func (entity *Account) Submit() bool {
+	if !entity.CanSubmit() {
+		return false
+	}
+
+	entity.Status = StatusSubmitted
+	return true
+}
+
+// Unsubmit will mark the account in a `INCOMPLETE` status. This will likely be for debugging purposes only.
+func (entity *Account) Unsubmit() {
+	entity.Status = StatusIncomplete
 }
 
-// Unlock will mark the account in an `unlocked` status.
-func (entity *Account) Unlock(context DatabaseService) error {
-	entity.Locked = false
-	_, err := entity.Save(context, entity.ID)
-	return err
+// CanKickback returns wether the account is in a valid state for kickback
+func (entity *Account) CanKickback() bool {
+	if entity.Status != StatusSubmitted {
+		return false
+	}
+	return true
+}
+
+// Kickback will mark the account in a `KICKBACK` status.
+func (entity *Account) Kickback() bool {
+	if !entity.CanKickback() {
+		return false
+	}
+
+	entity.Status = StatusKickback
+	return true
 }
 
 // FormTypeIsKnown returns wether the form type and version are known to eApp
@@ -197,7 +233,7 @@ func (entity *Account) BasicAuthentication(context DatabaseService, password str
 		entity.Token = basicMembership.Account.Token
 		entity.TokenUsed = basicMembership.Account.TokenUsed
 		entity.Email = basicMembership.Account.Email
-		entity.Locked = basicMembership.Account.Locked
+		entity.Status = basicMembership.Account.Status
 		entity.FormType = basicMembership.Account.FormType
 		entity.FormVersion = basicMembership.Account.FormVersion
 	}

api/admin/reject.go

@@ -25,9 +25,8 @@ func NewRejecter(db api.DatabaseService, store api.StorageService) Rejecter {
 
 // Reject rejects the application for a given account
 func (r Rejecter) Reject(account api.Account) error {
-	err := account.Unlock(r.db)
-	if err != nil {
-		return errors.Wrap(err, "Reject failed to unlock account")
+	if !account.CanKickback() {
+		return errors.New("Account can't be rejected if it hasn't been submitted")
 	}
 
 	// Load the application, clear the nos, save the application.
@@ -81,5 +80,16 @@ func (r Rejecter) Reject(account api.Account) error {
 		return errors.New(fmt.Sprintf("Got an error deleting %d documents: [%s]", len(deletionErrors), joinedErrs))
 	}
 
+	// Update the account status to "KICKBACK"
+	ok := account.Kickback()
+	if !ok {
+		return errors.New("The account got into a bad state during the rejection")
+	}
+
+	_, saveAccErr := account.Save(r.db, account.ID)
+	if saveAccErr != nil {
+		return errors.Wrap(saveAccErr, "couldn't save the account after changing the status")
+	}
+
 	return nil
 }

api/admin/submit.go

@@ -27,18 +27,13 @@ func NewSubmitter(db api.DatabaseService, store api.StorageService, xml api.XMLS
 // FilesForSubmission returns the XML and any Attachments for submission.
 func (s Submitter) FilesForSubmission(accountID int) ([]byte, []api.Attachment, error) {
 
-	// check that the acount isn't locked
 	// Get the account information from the data store
 	account := api.Account{ID: accountID}
 	_, accountErr := account.Get(s.db, accountID)
 	if accountErr != nil {
 		return []byte{}, []api.Attachment{}, accountErr
 	}
 
-	if account.Locked {
-		return []byte{}, []api.Attachment{}, errors.New("Account is locked")
-	}
-
 	application, appErr := s.store.LoadApplication(accountID)
 	if appErr != nil {
 		return []byte{}, []api.Attachment{}, errors.Wrap(appErr, "Can't load applicaiton")

api/cmd/kickback/main.go

@@ -0,0 +1,22 @@
+package main
+
+import (
+	"github.com/18F/e-QIP-prototype/api"
+	"github.com/18F/e-QIP-prototype/api/admin"
+	"github.com/18F/e-QIP-prototype/api/cmd"
+	"github.com/18F/e-QIP-prototype/api/log"
+)
+
+func main() {
+	logger := &log.Service{Log: log.NewLogger()}
+	cmd.Command(logger, func(context api.DatabaseService, store api.StorageService, account *api.Account) {
+		rejector := admin.NewRejecter(context, store)
+
+		rejectErr := rejector.Reject(*account)
+		if rejectErr != nil {
+			logger.WarnError("Failed to kickback", rejectErr, api.LogFields{"account": account.Username})
+		} else {
+			logger.Warn("Account kicked back", api.LogFields{"account": account.Username})
+		}
+	})
+}

api/cmd/unlock/main.go

@@ -9,8 +9,9 @@ import (
 func main() {
 	logger := &log.Service{Log: log.NewLogger()}
 	cmd.Command(logger, func(context api.DatabaseService, store api.StorageService, account *api.Account) {
-		if err := account.Unlock(context); err != nil {
-			logger.WarnError("Failed to unlock account", err, api.LogFields{"account": account.Username})
+		account.Unsubmit()
+		if _, err := account.Save(context, account.ID); err != nil {
+			logger.WarnError("Failed to save unlocked account", err, api.LogFields{"account": account.Username})
 		} else {
 			logger.Warn("Account unlocked", api.LogFields{"account": account.Username})
 		}

api/http/attachment.go

@@ -83,8 +83,8 @@ func (service AttachmentSaveHandler) ServeHTTP(w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	// If the account is locked then we cannot proceed
-	if account.Locked {
+	// If the account is submitted then we cannot proceed
+	if account.Status == api.StatusSubmitted {
 		service.Log.Warn(api.AccountLocked, api.LogFields{})
 		RespondWithStructuredError(w, api.AccountLocked, http.StatusForbidden)
 		return
@@ -239,7 +239,7 @@ func (service AttachmentDeleteHandler) ServeHTTP(w http.ResponseWriter, r *http.
 	}
 
 	// If the account is locked then we cannot proceed
-	if account.Locked {
+	if account.Status == api.StatusSubmitted {
 		service.Log.Warn(api.AccountLocked, api.LogFields{})
 		RespondWithStructuredError(w, api.AccountLocked, http.StatusForbidden)
 		return

api/http/form.go

@@ -33,7 +33,7 @@ func (service FormHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// If the account is locked then we cannot proceed
-	if account.Locked {
+	if account.Status == api.StatusSubmitted {
 		service.Log.Warn(api.AccountLocked, api.LogFields{})
 		RespondWithStructuredError(w, api.AccountLocked, http.StatusForbidden)
 		return

api/http/save.go

@@ -31,7 +31,7 @@ func (service SaveHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// If the account is locked then we cannot proceed
-	if account.Locked {
+	if account.Status == api.StatusSubmitted {
 		service.Log.Warn(api.AccountLocked, api.LogFields{})
 		RespondWithStructuredError(w, api.AccountLocked, http.StatusForbidden)
 		return

api/http/status.go

@@ -20,7 +20,7 @@ type StatusHandler struct {
 // formStatusInfo represents extra information associated with the application
 // regarding its current state.
 type formStatusInfo struct {
-	Locked bool
+	Status string
 	Hash   string
 }
 
@@ -57,7 +57,7 @@ func (service StatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	status := formStatusInfo{
-		Locked: account.Locked,
+		Status: account.Status,
 		Hash:   hash,
 	}
 

api/http/submit.go

@@ -33,7 +33,7 @@ func (service SubmitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// If the account is locked then we cannot proceed
-	if account.Locked {
+	if account.Status == api.StatusSubmitted {
 		service.Log.Warn(api.AccountLocked, api.LogFields{})
 		RespondWithStructuredError(w, api.AccountLocked, http.StatusForbidden)
 		return
@@ -47,7 +47,14 @@ func (service SubmitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Lock the account
-	if err := account.Lock(service.Database); err != nil {
+	ok := account.Submit()
+	if !ok {
+		service.Log.Warn(api.AccountUpdateError, api.LogFields{"status": account.Status})
+		RespondWithStructuredError(w, api.AccountUpdateError, http.StatusInternalServerError)
+		return
+	}
+
+	if _, err := account.Save(service.Database, account.ID); err != nil {
 		service.Log.WarnError(api.AccountUpdateError, err, api.LogFields{})
 		RespondWithStructuredError(w, api.AccountUpdateError, http.StatusInternalServerError)
 		return

api/integration/application_status_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	"github.com/18F/e-QIP-prototype/api"
 	"github.com/18F/e-QIP-prototype/api/http"
 )
 
@@ -151,7 +152,7 @@ func TestLockedStatus(t *testing.T) {
 	}
 
 	var status struct {
-		Locked bool
+		Status string
 		Hash   string
 	}
 
@@ -160,8 +161,8 @@ func TestLockedStatus(t *testing.T) {
 		t.Fatal(jsonErr)
 	}
 
-	if status.Locked != true {
-		t.Log("The account should not be locked")
+	if status.Status != api.StatusSubmitted {
+		t.Log("The account should be locked:", status.Status)
 		t.Fail()
 	}
 

api/integration/clear_yes_no_test.go

@@ -21,6 +21,14 @@ func TestClearEmptyAccount(t *testing.T) {
 	services := cleanTestServices(t)
 	account := createTestAccount(t, services.db)
 
+	// Hacky, but seems OK for these tests. Technically you shouldn't be able to submit
+	// anything but a complete application, but I think it's ok to make these tests smaller.
+	account.Submit()
+	_, saveErr := account.Save(services.db, account.ID)
+	if saveErr != nil {
+		t.Fatal(saveErr)
+	}
+
 	rejector := admin.NewRejecter(services.db, services.store)
 
 	err := rejector.Reject(account)
@@ -107,6 +115,14 @@ func rejectSection(t *testing.T, services serviceSet, json []byte, sectionName s
 		t.Fatal("Failed to save JSON", resp.StatusCode)
 	}
 
+	// Hacky, but seems OK for these tests. Technically you shouldn't be able to submit
+	// anything but a complete application, but I think it's ok to make these tests smaller.
+	account.Submit()
+	_, saveErr := account.Save(services.db, account.ID)
+	if saveErr != nil {
+		t.Fatal(saveErr)
+	}
+
 	rejector := admin.NewRejecter(services.db, services.store)
 	err := rejector.Reject(account)
 	if err != nil {
@@ -1036,6 +1052,14 @@ func TestClearComplexSectionNos(t *testing.T) {
 				t.Fatal("Failed to save JSON", resp.StatusCode)
 			}
 
+			// Hacky, but seems OK for these tests. Technically you shouldn't be able to submit
+			// anything but a complete application, but I think it's ok to make these tests smaller.
+			account.Submit()
+			_, saveErr := account.Save(services.db, account.ID)
+			if saveErr != nil {
+				t.Fatal(saveErr)
+			}
+
 			rejector := admin.NewRejecter(services.db, services.store)
 			err := rejector.Reject(account)
 			if err != nil {

api/integration/helpers.go

@@ -103,7 +103,7 @@ func createLockedTestAccount(t *testing.T, db api.DatabaseService) api.Account {
 		Email:       email,
 		FormType:    "SF86",
 		FormVersion: "2017-07",
-		Locked:      true,
+		Status:      api.StatusSubmitted,
 		ExternalID:  uuid.New().String(),
 	}
 
@@ -125,6 +125,7 @@ func createTestAccount(t *testing.T, db api.DatabaseService) api.Account {
 		Email:       email,
 		FormType:    "SF86",
 		FormVersion: "2017-07",
+		Status:      api.StatusIncomplete,
 		ExternalID:  uuid.New().String(),
 	}
 

api/integration/reject_test.go

@@ -78,6 +78,12 @@ func TestDeleteSingaturePDFs(t *testing.T) {
 		t.Fatal("submit didn't succeed")
 	}
 
+	// reload account now that it's been submitted.
+	loadErr := account.Find(services.db)
+	if loadErr != nil {
+		t.Fatal(loadErr)
+	}
+
 	// Reject this submission
 	rejector := admin.NewRejecter(services.db, services.store)
 	err := rejector.Reject(account)

api/migrations/20190623111111_create_form_status.sql

@@ -0,0 +1,18 @@
+
+-- +goose Up
+-- SQL in section 'Up' is executed when this migration is applied
+-- +goose StatementBegin
+ALTER TABLE accounts ADD COLUMN status text NOT NULL DEFAULT 'INCOMPLETE';
+UPDATE accounts SET status = 'SUBMITTED' WHERE locked = true;
+ALTER TABLE accounts ALTER COLUMN status DROP DEFAULT;
+ALTER TABLE accounts DROP COLUMN locked;
+-- +goose StatementEnd
+
+-- +goose Down
+-- SQL section 'Down' is executed when this migration is rolled back
+-- +goose StatementBegin
+ALTER TABLE accounts ADD COLUMN locked bool NOT NULL DEFAULT false;
+UPDATE accounts SET locked = true WHERE status = 'SUBMITTED';
+ALTER TABLE accounts ALTER COLUMN locked DROP DEFAULT;
+ALTER TABLE accounts DROP COLUMN status;
+-- +goose StatementEnd

api/postgresql/db_test.go

@@ -33,6 +33,7 @@ func TestAccountPersistence(t *testing.T) {
 		Email:       "[email protected]",
 		FormType:    "SF86",
 		FormVersion: "2017-07",
+		Status:      api.StatusIncomplete,
 		ExternalID:  uuid.New().String(),
 	}