msteele/APPEALS-24727 #19187

msteele96 · 2023-08-14T18:54:04Z

Description

Added validation to ensure that a given document series id exists within VBMS eFolder before allowing a HPR task to be created.

Acceptance Criteria

The value of the field labeled "Insert Caseflow Reader document hyperlink to request a hearing withdrawal" with be considered valid if:

The URL is of the following formatting:
https://vefs-claimevidence-ui-..bip.va.gov/file/<document_series_reference_id of document>
The document_series_reference_id is found to correspond with a document in eFolder via a request to one of their endpoints.
The validation should take place onChange, but debounced. This meaning that validation logic should be executed X number of milliseconds after a user stops entering text into the field. Example
If validation fails then error text should be displayed above the field and the field should be highlighted in red.
Error for whenever the URL formatting is invalid: Please enter a valid document hyperlink
Error for whenever the URL does not correspond with a document in eFolder: Document could not be located in eFolder
Refer to this Slack thread for further context: https://benefits-int-delivery.slack.com/archives/C03NCPYRXK2/p1687881917481399?thread_ts=1687878651.089549&cid=C03NCPYRXK2

Testing Plan

For feature branches merging into master: Was this deployed to UAT?

Frontend

User Facing Changes

Screenshots of UI changes added to PR & Original Issue

Best practices

Code Documentation Updates

Add or update code comments at the top of the class, module, and/or component.

Tests

Test Coverage

Did you include any test coverage for your code? Check below:

RSpec
Jest
Other

Code Climate

Your code does not add any new code climate offenses? If so why?

No new code climate issues added

Error Handling

Are errors being caught and handled gracefully?
Are appropriate error messages being displayed to users?
Are critical errors being reported to an error tracking system (e.g., Sentry, ELK)?
Are unhandled exceptions being caught at the application level ?

Exception Handling

Are custom exceptions defined and used where appropriate?
Is exception handling consistent throughout the codebase?
Are exceptions logged with relevant context and stack trace information?
Are exceptions being grouped and categorized for easier analysis and resolution?

codeclimate · 2023-08-14T19:01:28Z

Code Climate has analyzed commit 7878988 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1
Style	1

Note: there is 1 critical issue.

View more on Code Climate.

…t just present

jimfoley25 · 2023-08-23T13:21:29Z

app/controllers/appeals_controller.rb

@@ -92,6 +92,18 @@ def document_count
    handle_non_critical_error("document_count", error)
  end

+  # series_id is lowercase, no curly braces because it comes from url
+  def document_lookup
+    series_id = "{#{params[:series_id]}}".upcase


series_id = params[:series_id].to_s.upcase

This was from last story, but the curly braces are necessary for the lookup

jimfoley25 · 2023-08-23T13:26:05Z

app/controllers/appeals_controller.rb

+  # series_id is lowercase, no curly braces because it comes from url
+  def document_lookup
+    series_id = "{#{params[:series_id]}}".upcase
+    document = Document.find_by(series_id: series_id, file_number: appeal.veteran_file_number)


appeal&.veteran_file_number

jimfoley25 · 2023-08-23T14:12:23Z

app/models/tasks/hearing_mail_tasks/hearing_postponement_request_mail_task.rb

+  def open_assign_hearing_disposition_task?
+    # ChangeHearingDispositionTask is a subclass of AssignHearingDispositionTask
+    disposition_task_names = [AssignHearingDispositionTask.name, ChangeHearingDispositionTask.name]
+    open_task = appeal.tasks.where(type: disposition_task_names).open.first


you can probably simplify this method by adding a scope method to the task model with something like scope ```
:open_disposition_task, -> {
tasks.where(type: [AssignHearingDispositionTask.name, ChangeHearingDispositionTask.name]).open.first
}

then just use `open_task = appeal.open_disposition_task`

jimfoley25 · 2023-08-23T14:51:11Z

app/repositories/task_action_repository.rb

@@ -18,8 +18,13 @@ def assign_to_organization_data(task, _user = nil)
    end

    def mail_assign_to_organization_data(task, user = nil)


you should probably be able to use instance_of in both places in this method instead of is_a to narrow the type expectation even further

jimfoley25 · 2023-08-23T15:25:59Z

client/app/components/DateSelector.jsx

+    const errorMsg = dateValidationError(value);
+
+    setDateError(errorMsg);
+    validateDate?.(value !== '' && errorMsg === null);


suggest removing the ? as chaining is not needed here

client/app/queue/components/EfolderUrlField.jsx

ThorntonMatthew · 2023-08-25T20:39:03Z

client/app/queue/components/EfolderUrlField.jsx

+  const checkIfDocumentExists = async () => {
+    setLoading(true);


Whenever the "Retry" button is pressed, will this function's invocation allow for the parent component's state to be updated?

I've just updated the onclick of the retry button with the rest of the functionality from the debounce

ThorntonMatthew · 2023-08-25T20:45:04Z

client/app/queue/CreateMailTaskDialog.jsx

            value={this.state.eFolderUrl}
+            valid={this.state.eFolderUrlValid}


Are these two props able to be removed?

ThorntonMatthew · 2023-08-25T20:46:20Z

client/app/queue/components/EfolderUrlField.jsx

+  appealId: PropTypes.string.isRequired,
  requestType: PropTypes.string,
  value: PropTypes.string,
-  errorMessage: PropTypes.string
+  errorMessage: PropTypes.string,
+  valid: PropTypes.bool


Suggested change

appealId: PropTypes.string.isRequired,

requestType: PropTypes.string,

value: PropTypes.string,

errorMessage: PropTypes.string

errorMessage: PropTypes.string,

valid: PropTypes.bool

appealId: PropTypes.string.isRequired,

requestType: PropTypes.string,

errorMessage: PropTypes.string

In case the value and valid props are no longer passed in.

…y button onclick

ThorntonMatthew

LGTM!

APPEALS-24727 Add helper methods and pseudocode

821d572

msteele96 marked this pull request as draft August 14, 2023 18:54

msteele96 and others added 18 commits August 14, 2023 15:38

APPEALS-24727 Fixed import statement

c17c473

APPEALS-24727 Pass external id into efolderurl field for api call

8366a26

APPEALS-24727 Refactor debounce

b1764f8

APPEALS-24727 Debounce working after useEffect

9e1b0a5

APPEALS-24727 Removed useCallback

e7fe1f0

APPEALS-24727 Added loading styling and prop

081ac75

APPEALS-24727 Valid prop is passed between field and modal properly

6cd7d5f

APPEALS-24727 Moved loading to state value

db73a28

APPEALS-24727 Added error messages and retry button

b039f78

APPEALS-24727 Removed code used for testing

0efb78d

APPEALS-24727 Update style to fix CC issue

1d881f0

APPEALS-24727 Update Jest test

e96754a

APPEALS-24727 Linting fix

c061300

Merge branch 'master' into msteele/APPEALS-24727

cceef2a

Merge branch 'master' into msteele/APPEALS-24727

0fb676d

APPEALS-24727 Update validateForm to make sure efolderurl is valid no…

a130646

…t just present

APPEALS-24727 Update wording of label and corresponding tests

610241e

APPEALS-24727 linting fix

f5aaee5

jimfoley25 self-requested a review August 23, 2023 13:19

Merge branch 'feature/APPEALS-21339' into msteele/APPEALS-24727

7878988

msteele96 changed the base branch from master to feature/APPEALS-21339 August 23, 2023 15:44

msteele96 changed the title ~~msteele/APPEALS 24727~~ msteele/APPEALS-24727 Aug 23, 2023

jimfoley25 reviewed Aug 23, 2023

View reviewed changes

msteele96 added 4 commits August 24, 2023 15:28

APPEALS-24727 refactor to fix bug, last jest test failing

005a886

APPEALS-24727 Fix jest test and remove console log

e4c65ec

APPEALS-24727 Fixed flickering

529d557

APPEALS-24727 Added ignore line for linter

4411683

Merge branch 'feature/APPEALS-21339' into msteele/APPEALS-24727

1e03765

ThorntonMatthew reviewed Aug 25, 2023

View reviewed changes

APPEALS-24727 Removed unnecessary props and comments. Refactored retr…

4595025

…y button onclick

ThorntonMatthew approved these changes Aug 25, 2023

View reviewed changes

ThorntonMatthew marked this pull request as ready for review August 31, 2023 14:38

ThorntonMatthew merged commit cfb6cd6 into feature/APPEALS-21339 Aug 31, 2023
21 of 28 checks passed

ThorntonMatthew deleted the msteele/APPEALS-24727 branch August 31, 2023 14:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

msteele/APPEALS-24727 #19187

msteele/APPEALS-24727 #19187

msteele96 commented Aug 14, 2023 •

edited

Loading

codeclimate bot commented Aug 14, 2023 •

edited

Loading

jimfoley25 Aug 23, 2023

msteele96 Aug 25, 2023

jimfoley25 Aug 23, 2023

jimfoley25 Aug 23, 2023

jimfoley25 Aug 23, 2023

jimfoley25 Aug 23, 2023

ThorntonMatthew Aug 25, 2023

msteele96 Aug 25, 2023

ThorntonMatthew Aug 25, 2023

ThorntonMatthew Aug 25, 2023

ThorntonMatthew left a comment

		@@ -18,8 +18,13 @@ def assign_to_organization_data(task, _user = nil)
		end

		def mail_assign_to_organization_data(task, user = nil)

		const checkIfDocumentExists = async () => {
		setLoading(true);

		value={this.state.eFolderUrl}
		valid={this.state.eFolderUrlValid}

msteele/APPEALS-24727 #19187

msteele/APPEALS-24727 #19187

Conversation

msteele96 commented Aug 14, 2023 • edited Loading

Description

Acceptance Criteria

Testing Plan

Frontend

User Facing Changes

Best practices

Code Documentation Updates

Tests

Test Coverage

Code Climate

Error Handling

Exception Handling

codeclimate bot commented Aug 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThorntonMatthew left a comment

Choose a reason for hiding this comment

msteele96 commented Aug 14, 2023 •

edited

Loading

codeclimate bot commented Aug 14, 2023 •

edited

Loading