Add convert central to virtual and virtual to central hearings workflow #14704

sahalliburton · 2020-07-16T14:18:14Z

Resolves #14421

Description

This PR adds the convert-to-virtual workflow for Central Office hearings. This work is currently behind the feature flag schedule_virtual_hearings_for_central and offers users with this permission to change currently scheduled Central Office hearings to virtual hearings.

Acceptance Criteria

Code compiles correctly (tests pass etc.)
Storybook stories work as expected

Testing Plan

Central Office Hearing

Select a hearings admin user (BVASYELLOW)
Navigate to the hearings schedule and choose a 'Central' hearing from the list: http://localhost:3000/hearings/schedule
Click the 'Edit hearing details' link
In the hearing type dropdown choose 'Virtual'
Ensure the Hearing Conversion page matches the Figma mock from the ticket
Ensure that you can successfully convert back and forth between Virtual and Central

Video Hearing

Select a hearings admin user (BVASYELLOW)
Navigate to the hearings schedule and choose a 'Video' hearing from the list: http://localhost:3000/hearings/schedule
Click the 'Edit hearing details' link
In the hearing type dropdown choose 'Virtual'
Ensure the video hearing modal is displayed and all the functionality is unchanged
Ensure that you can successfully convert back and forth between Virtual and Video

User Facing Changes

Screenshots of UI changes added to PR & Original Issue

BEFORE	AFTER
-
-

Code Documentation Updates

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

Storybook Story

For Frontend (Presentationa) Components

Add a Storybook file alongside the component file (e.g. create MyComponent.stories.js alongside MyComponent.jsx)
Give it a title that reflects the component's location within the overall Caseflow hierarchy
Write a separate story (within the same file) for each discrete variation of the component

…fairs/caseflow into shalliburton/14421-convert-to-virtual

va-bot · 2020-07-16T14:20:50Z

	2 Warnings
⚠️	This is a Big PR. Try to break this down if possible. Stacked pull requests encourage more detailed and thorough code reviews
⚠️	This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

sahalliburton · 2020-07-16T14:22:13Z

client/app/hearings/HearingsApp.jsx

@@ -75,9 +76,7 @@ export default class HearingsApp extends React.PureComponent {

  routeForHearingDetails = ({ match: { params }, history }) => (
    <HearingsUserContext.Provider value={this.userPermissionProps()}>
-      <HearingsFormContextProvider>


Moved the forms context into the DetailsContainer component to inherit state that we already have

sahalliburton · 2020-07-16T14:22:51Z

client/app/hearings/components/Details.jsx


-    return {
-      hearingDetailsForm: {


All of this state was moved into the HearingsFormContext

codeclimate · 2020-07-16T14:23:18Z

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

View more on Code Climate.

sahalliburton · 2020-07-16T14:23:49Z

client/app/hearings/components/Details.jsx

-          ...(virtualHearingForm || {})
-        }
+  const submit = async (editedEmails) => {
+    try {


convert to try-catch block with async/await to remove all of the callbacks

rubaiyat22

some comments for backend

app/models/hearing.rb

app/models/hearings/forms/base_hearing_update_form.rb

app/models/legacy_hearing.rb

rubaiyat22 · 2020-07-16T16:22:51Z

app/controllers/hearings/hearing_day_controller.rb

@@ -34,7 +34,7 @@ def index
  def show
    render json: {
      hearing_day: hearing_day.to_hash.merge(
-        hearings: hearing_day.hearings_for_user(current_user).map { |hearing| hearing.quick_to_hash(current_user.id) }
+        hearings: hearing_day.hearings_for_user(current_user).map { |hearing| hearing.to_hash(current_user.id) }


I have some concerns that this may affect the performance of DailyDocket as the quick_to_hash does not serialize things that DailyDocket does not need yet like appellant_email_address which make calls VACOLS
cc: @ferristseng

there will be a performance tradeoff whether we use the quick_to_hash here or make an additional call on the details page, this preloads data that will be needed to reduce the number of roundtrips

I ended up reverting this and removing the frontend caching for now as it was creating too much of a bottleneck on the daily docket page @rubaiyat22 and @ferristseng, we should create a separate ticket though to revisit frontend caching so that we can improve client-side performance

app/models/hearing.rb

app/serializers/hearings/hearing_serializer.rb

app/serializers/hearings/legacy_hearing_serializer.rb

ferristseng

Looks great! I think there are some small things that I noted in the feedback to fix (some of the out-of-date stuff might be mislabelled in that it is still relevant). Thank you for all the new tests and stories!

app/controllers/hearings/hearing_day_controller.rb

app/models/hearings/forms/base_hearing_update_form.rb

app/serializers/hearings/hearing_serializer.rb

app/serializers/hearings/legacy_hearing_serializer.rb

ferristseng · 2020-07-16T15:47:18Z

client/app/components/FormLabel.jsx

+import React from 'react';
+import PropTypes from 'prop-types';
+
+export const FormLabel = ({ label, name, required, optional }) => (


Nice! Good refactors 🛠️

client/app/hearings/constants.js

client/app/hearings/containers/DetailsContainer.jsx

client/app/hearings/components/details/VirtualHearingForm.stories.js

ferristseng · 2020-07-17T15:23:51Z

client/test/app/hearings/components/Details.test.js

+      0
+    );
+
+    expect(details).toMatchSnapshot();


Great tests!!

rubaiyat22

Some differences I noticed between design and my dev. let me know if you wanna talk about any of these.

Text under the hearing time dropdown is missing
Figma:
if have some unsaved on details page and I choose to convert a central to Virtual, it does not indicate that there some unsaved changes (Maybe be out of scope for this PR as we did not decide to go with a new page. If out of scope, me may want to write that somwhere/create a ticket)
POA email is being validated when converting to virtual .

Also the convert button changes to disabled loading button and does not allow me to make any edits without reloading the page
4. Updating the timezone for Veteran the modal looks like this

but in figma it looks like this

this also applies to the success message which in figma looks like this

Upading the timezone for a poa looks the same as update timezone for veteran

but in figma it looks like this
changing timezone for both appellant and veteran also looks the same as above

But in figma it looks like this:

rubaiyat22 · 2020-07-16T19:14:17Z

client/app/hearings/components/Details.jsx

+
+/**
+ * Hearing Details Component
+ * @param {Object} props -- React props inherited from client/app/hearings/containers/DetailsContainer.jsx


Nice and clear comment!

rubaiyat22 · 2020-07-16T19:32:32Z

client/app/hearings/components/Details.jsx

+        });
+
+        // Focus to the error
+        return document.getElementById('email-section').scrollIntoView();


rubaiyat22 · 2020-07-16T19:35:20Z

client/app/hearings/components/Details.jsx

+        // Set the Virtual Hearing errors
+        setVirtualHearingErrors({
+          [!hearing.virtualHearing.appellantEmail && 'appellantEmail']: 'Appellant email is required',
+          [!hearing.virtualHearing.representativeEmail && 'representativeEmail']: 'Representative email is required',


[note] I think there's a ticket out to remove this required constraint for representative email

rubaiyat22 · 2020-07-16T19:39:53Z

client/app/hearings/components/Details.jsx

@@ -3,253 +3,182 @@ import { connect } from 'react-redux';
 import { css } from 'glamor';
 import AppSegment from '@department-of-veterans-affairs/caseflow-frontend-toolkit/components/AppSegment';
 import PropTypes from 'prop-types';
-import React, { useState, useEffect, useContext } from 'react';
-import _ from 'lodash';


great idea to import specifics

rubaiyat22 · 2020-07-17T02:03:13Z

client/app/hearings/components/HearingConversion.jsx

+import { JudgeDropdown } from '../../components/DataDropdowns/index';
+import { marginTop, noMaxWidth } from './details/style';
+import COPY from '../../../COPY';
+import { AddressLine } from './details/Address';


Noticed that there is an Address component in /queue/components. This might be place where we could take it out to the shared components and reuse it but that's out of scope for this PR

rubaiyat22

Great job putting all these together. It looks great!!
It looks like we're sending confirmation emails to both appellant and poa when either of the timezone changes so i think we have to change our backend to send confirmation emails to only the recipient whose timezone was updated. I added some comments about some differences i found in figma and your PR hence requested changes but let me know if anything is too out of scope and you want to create a follow up ticket instead.
Also would you mind adding some screens shots of the changes in the PR.

rubaiyat22

Going to go ahead and approve as @sahalliburton and I resolved some of comments async and we created some follow up tickets

Stephen Halliburton added 23 commits June 24, 2020 10:05

WIP: Started adding convert to virtual workflow

ae5985f

WIP: Started changing vh attributes

05bca62

Started large refactor of details

46d4cab

WIP: Finished refactoring details component

ffd7903

Fixed merge conflicts

56d5d5c

Updated storybook stories

6c18c47

Separated conversion components and got conversion flow working

70d961c

Added timezone component

b28c8c6

Merge branch 'master' of https://github.com/department-of-veterans-af…

0dc0fa7

…fairs/caseflow into shalliburton/14421-convert-to-virtual

Finished adding convert between virtual/central workflow

3e9caa8

Consolidated new components for hearing conversion

e3a789b

Added stories for new components

213ac24

Added test stubs for components to be tested

19a2a24

Added timezone service for rep address and rep address

338ec09

Reset serializers

d5d0a72

Fixed merge conflicts

b2203c5

Fixed failing tests after merge to master

18f9f81

Fixed merge conflicts

e4bb04b

Updated unit tests for conversion components

da4e49b

Merge branch 'master' of https://github.com/department-of-veterans-af…

2e4ccb1

…fairs/caseflow into shalliburton/14421-convert-to-virtual

Finished adding tests for new components

ea26986

Merge branch 'master' of https://github.com/department-of-veterans-af…

3479701

…fairs/caseflow into shalliburton/14421-convert-to-virtual

Merged to amster

4cb8f80

sahalliburton requested review from ferristseng and rubaiyat22 and removed request for ferristseng July 16, 2020 14:18

sahalliburton commented Jul 16, 2020

View reviewed changes

Stephen Halliburton added 3 commits July 16, 2020 10:49

Refactored based on code climate

d360cff

Refactored based on code climate

461edad

Fixed timezone calculation for tests

76c861f

rubaiyat22 reviewed Jul 16, 2020

View reviewed changes

Stephen Halliburton added 8 commits July 16, 2020 12:43

Updated based on comments

c9472c7

Updated based on comments

92de6af

Moved timezone functions into appeal model

29169f0

Updated based on code climate

5cb406e

Fixed failing rspec tests

36a0534

Hide timezone for formerly video hearings

d10e7e4

Ignore codeclimate branch size

6f594b5

Fixed lint errors

8150c92

ferristseng approved these changes Jul 17, 2020

View reviewed changes

rubaiyat22 reviewed Jul 17, 2020

View reviewed changes

rubaiyat22 suggested changes Jul 17, 2020

View reviewed changes

Stephen Halliburton added 4 commits July 17, 2020 12:46

Started refactoring from feedback

8be58dc

Addressed feedback from PR

aba96ec

Added clear alert state when transitioning between pages

f7dcbbe

Refactored useEffect call

a824725

sahalliburton mentioned this pull request Jul 17, 2020

Update VirtualHearingModal for timezone and email changes #14730

Closed

3 tasks

rubaiyat22 approved these changes Jul 17, 2020

View reviewed changes

Removed validation for POA email on central

a91e115

sahalliburton added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Jul 20, 2020

va-bot added 2 commits July 20, 2020 08:30

Merge branch 'master' into shalliburton/14421-convert-to-virtual

c904d26

Merge branch 'master' into shalliburton/14421-convert-to-virtual

2b756c6

va-bot merged commit e8164bc into master Jul 20, 2020

va-bot deleted the shalliburton/14421-convert-to-virtual branch July 20, 2020 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add convert central to virtual and virtual to central hearings workflow #14704

Add convert central to virtual and virtual to central hearings workflow #14704

sahalliburton commented Jul 16, 2020

va-bot commented Jul 16, 2020 •

edited

Loading

sahalliburton Jul 16, 2020

sahalliburton Jul 16, 2020

codeclimate bot commented Jul 16, 2020 •

edited

Loading

sahalliburton Jul 16, 2020

rubaiyat22 left a comment

rubaiyat22 Jul 16, 2020

sahalliburton Jul 16, 2020

sahalliburton Jul 16, 2020

ferristseng left a comment

ferristseng Jul 16, 2020

ferristseng Jul 17, 2020

rubaiyat22 left a comment

rubaiyat22 Jul 16, 2020

rubaiyat22 Jul 16, 2020

rubaiyat22 Jul 16, 2020

rubaiyat22 Jul 16, 2020

rubaiyat22 Jul 17, 2020

rubaiyat22 left a comment •

edited

Loading

rubaiyat22 left a comment

Add convert central to virtual and virtual to central hearings workflow #14704

Add convert central to virtual and virtual to central hearings workflow #14704

Conversation

sahalliburton commented Jul 16, 2020

Description

Acceptance Criteria

Testing Plan

User Facing Changes

Code Documentation Updates

Storybook Story

va-bot commented Jul 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot commented Jul 16, 2020 • edited Loading

Choose a reason for hiding this comment

rubaiyat22 left a 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

ferristseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubaiyat22 left a 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

rubaiyat22 left a comment • edited Loading

Choose a reason for hiding this comment

rubaiyat22 left a comment

Choose a reason for hiding this comment

va-bot commented Jul 16, 2020 •

edited

Loading

codeclimate bot commented Jul 16, 2020 •

edited

Loading

rubaiyat22 left a comment •

edited

Loading