IFRCGo / go-frontend

MIT License
21 stars 5 forks source link

Refactor Field Report Form #1898

Closed frozenhelium closed 3 years ago

frozenhelium commented 3 years ago

Is your feature request related to a problem? Please describe. Most of the Field Report Form code is obsolete. It uses method of coding that was used few years ago. Since then, React itself has updated a lot and has introduced better and easier way, which allows better, cleaner and less error-prone coding. If the Field Report Form is left as-is, It would be a bit difficult and time consuming to add features or modify.

Describe the solution you'd like Refactor the Field Report Form to use better code, form validation

frozenhelium commented 3 years ago

Available for testing on: https://ifrc-go-feature-new-field-report-form.surge.sh/reports/new

batpad commented 3 years ago

@frozenhelium this is some really amazing work here.

@karitotp - would you be able to spend some time testing https://ifrc-go-feature-new-field-report-form.surge.sh/reports/new to create field reports, and verify that all the information on the created field report is correct? Will be good to try a few combinations of things, there likely will be minor issues.

batpad commented 3 years ago

@karitotp to note: @frozenhelium is working on fixing the visual / CSS issues to make it look like the previous form, but it will be great if you can test the functionality, the validation, saving, etc. Thanks!

batpad commented 3 years ago

@frozenhelium this one is a bit tricky:

The two fields Government Requires International Assistance and National Society Requests International Assistance need to be unselected by default and allow the user to provide a null response - i.e. choose neither Yes nor No. This is a bit weird the way it works currently, but I believe we need this functionality. We can also discuss in the group, but just noting down as a change of behaviour from the existing form.

karitotp commented 3 years ago

@frozenhelium @batpad, here my findings testing the creation of different Field Reports

Screenshot from 2021-05-20 09-40-13

Screenshot from 2021-05-20 09-02-36

Selection_002

In a FR no related to covid and a FR of Early Warning also there are numbers that are not been displayed.

Screenshot from 2021-05-20 10-21-24

Screenshot from 2021-05-20 10-24-44

frozenhelium commented 3 years ago

@karitotp Thanks a million for testing these out. I've just pushed fixes for all of the issues mentioned above except the following one:

  • In a event related to Covid, in situation tab, should the Source of figures and Date of data be required fields when the numeric fields is filled? If so, the validation of required fields is not working here.

Could you please take a look at it again? Thanks again!

karitotp commented 3 years ago

@frozenhelium, all the above things is working good now :+1:

frozenhelium commented 3 years ago

Summary of the refactor

Identified Report types (EW, EVT, EPI, COV)

Form Steps

Step1: Context

Step2: Risk Analysis / Situation

Step3: Early Action / Action

Step4: Response

Known issues in old system (Currently at staging server)

Notes for testing

tovari commented 3 years ago

When changing from Epidemic type of Event to EW/EA and back to Event, then a error message indicates the missing Disaster type. Disaster type should be cleared, but without error message. Error message should only appear when user tries to step to the next page. image

image

batpad commented 3 years ago

@karitotp could you give this another test, based on @frozenhelium's comment above?

karitotp commented 3 years ago

@batpad , all is working as @frozenhelium's comments and the field required message when a epidemic report is changed to Early warning reported by @tovari is still there.

tovari commented 3 years ago

@frozenhelium, here is a text duplication on Action tab, only 'Information Bulletin' should appear in bold text in the first line: image

tovari commented 3 years ago

@frozenhelium, noticed that some of the field values are not saved: Situation tab:

frozenhelium commented 3 years ago

When changing from Epidemic type of Event to EW/EA and back to Event, then a error message indicates the missing Disaster type. Disaster type should be cleared, but without error message. Error message should only appear when user tries to step to the next page. image

image

@tovari This is expected behaviour (i.e. we set the validation to immediately trigger). This behaviour is consistent with other places as well, where the validation is immediately triggered when related fields are changed. Let me know if we should change this behaviour.

batpad commented 3 years ago

@karitotp could you do one final test of the Field Report form functionality that everything looks good after all the latest changes?

anamariaescobar commented 3 years ago

Some feedback from the UAT regarding Field Reports:

karitotp commented 3 years ago

@batpad this is what I found

Selection_001

frozenhelium commented 3 years ago

@anamariaescobar Thanks for the catch. I've fixed the "No" not being showed for the "International assistance requested". For the Epidemic FR actions taken by the IFRC, it appears different in the staging because it's loaded from the API. It'll be same as other 2 when it goes to production. I verified this in my local setup using the prod API.