search

department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
283 stars 204 forks source link

[Spike] Identify and verify unused files in the VAOS application #84657

Closed jenniemc closed 1 month ago

jenniemc commented 5 months ago

Background/Request

During the Awesome Font icon conversion, there are several files that doesn't seem be to utilized in the latest version of vaos app. While the happy path was employed to separate the used files, the unused files may be utilized in outlier situations.

Below are files that require verification as un-used:

Goal

Identify and verify unused files in the VAOS application. Some files may be abandoned during prototyping or obsolete because of deleted feature flag association.

Requirements to Consider

The some files may be used in fringe situations, test those outlier paths.

Tasks

Time Box

_ hours

Definition of Done

Bren22va commented 3 months ago

Hey team! Please add your planning poker estimate with Zenhub @cferris32 @jenniemc @JunTaoLuo @ryanshaw @simiadebowale @vbahinwillit

ryanshaw commented 1 month ago

Files Mentioned in this Spike

src/applications/vaos/new-appointment/components/ConfirmationPage/index.jsx

This file is currently serving as a wrapper for the ConfirmationDirectScheduleInfoV2.jsx component and cannot be removed without some simple refactoring.

My suggestion would be to consolidate this within a single ConfirmationPage component.

src/applications/vaos/appointment-list/components/AppointmentsPage/AppointmentListItemGroup.jsx

This file is not imported in any other component or test, it can be removed. After deletion locally, all unit tests and e2e tests passed. A search of the codebase revealed that it was not included in any other file.

src/applications/vaos/appointment-list/components/AppointmentsPage/RequestListItem.jsx

This file is not imported in any other component or test, it can be removed. After deletion locally, all unit tests and e2e tests passed. A search of the codebase revealed that it was not included in any other file.

src/applications/vaos/appointment-list/components/RequestedAppointmentsList.jsx

While this component is not used, there are some existing unit tests that should be updated to use the more current RequestedAppointmentsListGroup component before this component can be removed.

Test assertions that should be up for consideration to update and potentially move are:

Unused Feature Flag

vaOnlineSchedulingBreadcrumbUrlUpdate feature flag and code removal

We no longer use this feature flag, however it is present in ~44 files. Removing this could be a large effort, but in the long run will remove a lot of unused code. There are places where test coverage can be removed and also places where test coverage needs to be added -- where we are only testing cod that is not used in the application.

My suggestion would be to make this a single large issue with many smaller pull requests or create issues by upper level directories in the application (i.e. remove from new-appointment/, remove from appointment_list/, etc.)

I can make new tech debt issues for the files that can be easily removed above as well as the feature flag removal.

Static Code Analysis

I also ran a static code analyzer that detects unused React components: 🔍 findead | Findead

Ran findead on src/appplications/vaos/ with this command: findead ./src/applications/vaos

Here are the results:


===================== Findead is looking for components... =====================

                PreferredProviderSection | ./src/applications/vaos/new-appointment/components/ReviewPage/PreferredProviderSection.jsx | 3 KB
                          LoadingOverlay | ./src/applications/vaos/components/LoadingOverlay.jsx | 546 Bytes
                   CernerTransitionAlert | ./src/applications/vaos/components/CernerTransitionAlert.jsx | 2 KB
                                    When | ./src/applications/vaos/components/layout/DetailPageLayout.jsx | 4 KB
                                    What | ./src/applications/vaos/components/layout/DetailPageLayout.jsx | 4 KB
                                     Who | ./src/applications/vaos/components/layout/DetailPageLayout.jsx | 4 KB
                                   Where | ./src/applications/vaos/components/layout/DetailPageLayout.jsx | 4 KB
                                 Prepare | ./src/applications/vaos/components/layout/DetailPageLayout.jsx | 4 KB
                         VAOSBreadcrumbs | ./src/applications/vaos/components/Breadcrumbs.jsx | 2 KB
                     AppointmentDateTime | ./src/applications/vaos/appointment-list/components/AppointmentDateTime.jsx | 1 KB
                     NoOnlineCancelAlert | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/NoOnlineCancelAlert.jsx | 2 KB
                              TypeHeader | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/TypeHeader.jsx | 668 Bytes
                   VideoInstructionsLink | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VideoInstructionsLink.jsx | 854 Bytes
                          CCInstructions | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/CCInstructions.jsx | 785 Bytes
                            CalendarLink | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/CalendarLink.jsx | 1 KB
                      VideoVisitProvider | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VideoVisitProvider.jsx | 1 KB
                           VideoLocation | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VideoLocation.jsx | 2 KB
                       PhoneInstructions | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/PhoneInstructions.jsx | 685 Bytes
                               PrintLink | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/PrintLink.jsx | 846 Bytes
                          VAInstructions | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VAInstructions.jsx | 1 KB
                              CancelLink | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/CancelLink.jsx | 2 KB
                 RescheduleOrCancelAlert | ./src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/RescheduleOrCancelAlert.jsx | 782 Bytes
                    RequestedStatusAlert | ./src/applications/vaos/appointment-list/components/RequestedStatusAlert.jsx | 3 KB
                         RequestListItem | ./src/applications/vaos/appointment-list/components/AppointmentsPage/RequestListItem.jsx | 3 KB
                AppointmentListItemGroup | ./src/applications/vaos/appointment-list/components/AppointmentsPage/AppointmentListItemGroup.jsx | 7 KB
                 PastAppointmentsListNew | ./src/applications/vaos/appointment-list/components/PastAppointmentsList/index.jsx | 7 KB
                  AppointmentListSection | ./src/applications/vaos/appointment-list/index.jsx | 3 KB
=================================== Results ====================================
======================== 27 possible dead components :/ ========================
===================      278 browsed files in 13.651 seconds ===================

I've gone through each file and noted when there were false positives. The general methodology was to:

src/applications/vaos/new-appointment/components/ReviewPage/PreferredProviderSection.jsx

This file is not imported in any other component or test, it can be removed. After deletion locally, all unit tests and e2e tests passed. A search of the codebase revealed that it was not included in any other file.

src/applications/vaos/components/LoadingOverlay.jsx

This file is not imported in any other component or test, it can be removed. After deletion locally, all unit tests and e2e tests passed. A search of the codebase revealed that it was not included in any other file.

src/applications/vaos/components/CernerTransitionAlert.jsx

The only use of this component in the codebase is commented out and located here: vets-website/src/applications/vaos/appointment-list/components/AppointmentsPage/index.jsx at 42e16f044bc5f4e9484e4d1569b85c954ae35a1c · department-of-veterans-affairs/vets-website · GitHub

If we are no longer needing this particular piece of functionality we should delete the file and remove the comments. It is related to this PR: #78946: Remove Code to Prevent Lovell Appointment booking (#28920) · department-of-veterans-affairs/vets-website@0cbab09 · GitHub

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/NoOnlineCancelAlert.jsx

This alert component was used in the old DetailsVA component, which was replaced with the new appointment redesign effort and is no longer needed. It is safe to remove the unit test coverage for this component as well: NoOnlineCancelAlert.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/TypeHeader.jsx

This was part of a Community Care Detail Page refactor from two years ago and is no longer needed or used. We can safely delete this and the associated unit test:TypeHeader.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VideoInstructionsLink.js

This component is no longer used and can be deleted. The associated unit test can be removed as well: VideoInstructionsLink.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/CCInstructions.jsx

This component is no longer used and can be deleted. The associated unit test can be removed as well: CCInstructions.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/CalendarLink.jsx

This component is no longer used and can be deleted. There are no associated tests.

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VideoVisitProvider.jsx

This component is no longer used and can be deleted. The associated unit test can be removed as well: VideoVisitProvider.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VideoLocation.jsx

This component is no longer used and can be deleted. There are no associated tests.

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/PhoneInstructions.jsx

This component is no longer used and can be deleted. There are no associated tests.

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/PrintLink.jsx

This component is no longer used and can be deleted. The associated unit test can be removed as well: PrintLink.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/VAInstructions.jsx

This component is no longer used and can be deleted. The associated unit test can be removed as well: VAInstructions.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/CancelLink.jsx

This component is no longer used and can be deleted. The associated unit test can be removed as well: CancelLink.unit.spec.js

src/applications/vaos/appointment-list/components/ConfirmedAppointmentDetailsPage/RescheduleOrCancelAlert.jsx

This was part of a Community Care Detail Page refactor from two years ago and is no longer needed or used. We can safely delete this. There are no associated unit tests.

src/applications/vaos/appointment-list/components/RequestedStatusAlert.jsx

This component is no longer used and can be deleted. The associated unit test can be removed as well: RequestedStatusAlert.unit.spec.js

src/applications/vaos/appointment-list/components/PastAppointmentsList/index.jsx

This was a false positive.

src/applications/vaos/appointment-list/index.jsx

This was a false positive.

Inline components in /src/applications/vaos/components/layout/DetailPageLayout.jsx

These were all false positives.

JunTaoLuo commented 1 month ago

Other than the files listed in the original description, are there any files and components that are not used in the VAOS app? I recall you mentioned that there are some tools that can help identify them.

ryanshaw commented 1 month ago

@JunTaoLuo I've updated my comments, there were a few more things that we should consider removing. I didn't note it in the ticket, but I did run another static analysis of the codebase that looked at functions and uncalled code in general (versus module imports) and there is a moderate to small amount of that.

cc: @simiadebowale

ryanshaw commented 1 month ago

@simiadebowale I wanted to call out this bit, as you worked on it and I don't have context:

src/applications/vaos/components/CernerTransitionAlert.jsx

The only use of this component in the codebase is commented out and located here: vets-website/src/applications/vaos/appointment-list/components/AppointmentsPage/index.jsx at 42e16f044bc5f4e9484e4d1569b85c954ae35a1c · department-of-veterans-affairs/vets-website · GitHub

If we are no longer needing this particular piece of functionality we should delete the file and remove the comments. It is related to this PR: #78946: Remove Code to Prevent Lovell Appointment booking (#28920) · department-of-veterans-affairs/vets-website@0cbab09 · GitHub

JunTaoLuo commented 1 month ago

Thanks @ryanshaw for the detailed investigation! It now covers all the files I noticed when I manually looked for unused components so I'm glad the static analysis tool worked well. Looking forward to deleting all this dead code 😄 .

ldelacosta commented 1 month ago

Team has identified the work that is needed to clean up the unused files in the Appointments tool.

92985

92986

92987

92988

92990

92991

92992

92993

92994

93064

No further analysis needed. Closing out the ticket.