Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

feat(incidents): add ability to resolve incidents #2222

Merged
merged 17 commits into from
Jul 12, 2020
Merged

feat(incidents): add ability to resolve incidents #2222

merged 17 commits into from
Jul 12, 2020

Conversation

blestab
Copy link
Contributor

@blestab blestab commented Jul 10, 2020

Fixes #2078.
Fixes #2218.

Changes proposed in this pull request:

Incident View

  • Add new user permission CompleteIncident
  • Add new incident property CompletedOn
  • Add new incident status completed
  • Add Complete Incident button on ViewIncident screen
  • Add Completed On display information on the ViewIncident screen for a resolved incident
  • Display Reported By username correctly
  • Unit Tests
    -- Show Complete Incident button when incident is not completed
    -- Show Complete Incident button when user has the access
    -- Do not show the Complete button when incident is already completed
    -- Do not show the Complete button if user does not have the access
    -- Show the Completed On details if the incident is completed
    -- Do not show the Completed On details if the incident is not completed
    -- Extract username utility unit test

Incidents View

  • Display Reported By username correctly
  • Add completed status to list of filter options

Incidents Utilities

  • Utility to extract username from the couchdb user id string

@gitpod-io
Copy link

gitpod-io bot commented Jul 10, 2020

@vercel
Copy link

vercel bot commented Jul 10, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hospitalrun/hospitalrun-frontend/7jw83ca31
✅ Preview: https://hospitalrun-frontend-git-fork-blestab-master.hospitalrun.vercel.app

@blestab blestab changed the base branch from master to oizuldan/master July 10, 2020 09:44
@blestab blestab changed the base branch from oizuldan/master to master July 10, 2020 09:44
@blestab
Copy link
Contributor Author

blestab commented Jul 10, 2020

@jackcmeyer @kumikokashii @tehkapa sorry to bug, may i have some guidance/suggestions regarding items [Q1] and [Q2] in my draft PR.

@fox1t
Copy link
Member

fox1t commented Jul 10, 2020

As for Q1 you are trying to modify in place an object that is not extensible
try to write it like

  const completedIncident = await IncidentRepository.saveOrUpdate({
    ...incidentToComplete,
    completedOn: new Date(Date.now().valueOf()).toISOString(),
    status: 'completed',
  })

You cannot modify objects in place: you have to "generate" a new object and copy props from the old one.

Q2 seems that it need a fix. You can open an another PR if you want to work on it :)

@blestab
Copy link
Contributor Author

blestab commented Jul 10, 2020

Thanks @fox1t i could have sworn that i tried this before and it refused to work 😇 , works this time around though so that one is now fixed, thanks.

I have also fixed Q2 as part of this PR so will update the description.

Just need to add some tests then i can remove it from draft status

@blestab blestab marked this pull request as ready for review July 10, 2020 18:17
@blestab
Copy link
Contributor Author

blestab commented Jul 10, 2020

Hi all,
PR is now ready for review

@jackcmeyer jackcmeyer changed the title Add ability to resolve incidents feat(incidents): add ability to resolve incidents Jul 11, 2020
@jackcmeyer jackcmeyer requested review from jackcmeyer, a user and fox1t July 11, 2020 04:00
@jackcmeyer jackcmeyer added incidents 🚀enhancement an issue/pull request that adds a feature to the application labels Jul 11, 2020
@jackcmeyer jackcmeyer added this to the v2.0 milestone Jul 11, 2020
Copy link
Member

@jackcmeyer jackcmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few code related comments.

overall, everything looks good.

One thing, I think we should rename everything that says complete to be resolve. It seems like resolve would be the event a user would take when a incident is no longer an incident.

src/incidents/util/extractUsername.ts Outdated Show resolved Hide resolved
src/incidents/incident-slice.ts Outdated Show resolved Hide resolved
src/__tests__/incidents/util/extract-username-util.test.ts Outdated Show resolved Hide resolved
src/__tests__/incidents/util/extract-username-util.test.ts Outdated Show resolved Hide resolved
@blestab
Copy link
Contributor Author

blestab commented Jul 11, 2020

Thanks @jackcmeyer
I have made code changes as suggested.

@jackcmeyer jackcmeyer merged commit 5e95a9c into HospitalRun:master Jul 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🚀enhancement an issue/pull request that adds a feature to the application incidents
Projects
None yet
4 participants