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

Incident - reportedBy should store the currently logged in user id #2539

Open
blestab opened this issue Jan 4, 2021 · 27 comments 路 May be fixed by #2576, #2810 or #2988
Open

Incident - reportedBy should store the currently logged in user id #2539

blestab opened this issue Jan 4, 2021 · 27 comments 路 May be fixed by #2576, #2810 or #2988
Assignees
@xhefribala
Labels
馃悰bug issue/pull request that documents/fixes a bug good first issue indicates an issue is good for a first time contributor in progress indicates that issue/pull request is currently being worked on incidents
Projects
Version 2.0
Milestone
v2.0

Comments

@blestab
Copy link
Contributor

blestab commented Jan 4, 2021

馃悰 Bug Report

Report incident is currently hardcoded to set reportedBy to some user instead of the currently logged in user's id. This needs to be corrected to user the current user id from the redux store.

To Reproduce

Expected behavior

  • The incident.reportedBy should be the currently logged in user.id at the time the incident is reported
  • The list of incidents should display the user.fullName based on the incident.reportedBy

Notes

  • See src/shared/components/navbar/Navbar.tsx on how to access the user from the store

Your Environment

  • node version: 6,8,10
  • fastify version: >=1.0.0
  • os: Mac, Windows, Linux
  • any other relevant information
@blestab blestab added 馃悰bug issue/pull request that documents/fixes a bug good first issue indicates an issue is good for a first time contributor incidents labels Jan 4, 2021
@blestab blestab added this to the v2.0 milestone Jan 4, 2021
@blestab blestab added this to To do in Version 2.0 via automation Jan 4, 2021
@jameszheng405
Copy link

jameszheng405 commented Jan 4, 2021 •
edited

@blestab @jackcmeyer @morrme @TheKappa @fox1t Hi, this is James from Stanford Code the Change! I'd like to be assigned to this task

@morrme morrme added the in progress indicates that issue/pull request is currently being worked on label Jan 4, 2021
@morrme
Copy link
Member

morrme commented Jan 4, 2021

@jameszheng405 it's yours!

@blestab
Copy link
Contributor Author

blestab commented Jan 14, 2021

Hi @jameszheng405,
How is this coming along? Feel free to check this PR #2544 for ideas

@jameszheng405
Copy link

Hi @blestab ,
I'm able to display the user id now, just working on displaying the full name in the table. I'll be meeting with Stanford CTC tomorrow to discuss this. Thanks for checking in!

@blestab
Copy link
Contributor Author

blestab commented Jan 15, 2021

Hi @blestab ,
I'm able to display the user id now, just working on displaying the full name in the table. I'll be meeting with Stanford CTC tomorrow to discuss this. Thanks for checking in!

Awesome! Thanks

@jameszheng405
Copy link

@blestab
After taking a look at PR #2544, I noticed that fullName is statically defined and used to test the value of requestedByFullName. I was wondering if this might be inefficient in case a user decides to change their name and the value of fullName would then have to be changed everywhere? I鈥檓 thinking it would be better to always be able to access the current fullName through the user id, which never changes. However, since that information isn鈥檛 globally available yet, I would like to propose an extractFullName function which will be scaffolded later when that functionality is there.

@riiniii
Copy link
Contributor

riiniii commented Jan 16, 2021

that's a good point. currently doesn't look like we have functionality to change much about user, but is good for if/when user info editing is implemented for the future..

following + will make changes as necessary for #2544

jameszheng405 added a commit to codethechange/hospitalrun-frontend that referenced this issue Feb 21, 2021
@jameszheng405
fix(fullname): added extractFullName() template
2a0ecfb 
fix HospitalRun#2539
@jameszheng405 jameszheng405 linked a pull request Feb 21, 2021 that will close this issue
Open
@sshikhar1234
Copy link

Hi is this issue fixed in production?

@matteovivona
Copy link
Contributor

@sshikhar1234 which production?

@sshikhar1234
Copy link

I mean like in main release. Is this issue fixed or its open?

@matteovivona
Copy link
Contributor

No, if this issue is still open, I don't think so. Why?

jameszheng405 added a commit to codethechange/hospitalrun-frontend that referenced this issue Apr 18, 2021
@jameszheng405
fix(extractfullname.ts): scaffolds extractFullName()
85414d5 
Previously, the extractUserName() function was returning a mocked "some user." We scaffolded out
this function for when we complete the user model.

fix HospitalRun#2539

fixed extractFullName import

remove reportedBy

make app test less spurious
@akshatsinghania
Copy link

i would like to contribute fixing this issue , should i submit a pull request?

@bottercode
Copy link

i would like to work on this one. Is it still Open?

@matteovivona
Copy link
Contributor

yep @bottercode

@bottercode
Copy link

I have fixed the issue, new to this so can you just tell me if you have received the pull request i made?

@matteovivona
Copy link
Contributor

I have fixed the issue, new to this so can you just tell me if you have received the pull request i made?

@bottercode I don't see any new PR https://github.com/HospitalRun/hospitalrun-frontend/pulls. Can you link it?

@bottercode
Copy link

Sorry for the very late response here it is @tehkapa bottercode@d327ac1

@matteovivona
Copy link
Contributor

@bottercode you need to do a PR in hospitalrun/hospitalrun-frontend and not on your repo

@bottercode
Copy link

Is it visible now? @tehkapa

@matteovivona
Copy link
Contributor

Is it visible now? @tehkapa

No. You can see all open PR here https://github.com/HospitalRun/hospitalrun-frontend/pulls

@Rockingrajat
Copy link

@tehkapa can you please assign this to me? I was able to reproduce the issue and going through the code which handles the reported incidents.

@matteovivona
Copy link
Contributor

@tehkapa can you please assign this to me? I was able to reproduce the issue and going through the code which handles the reported incidents.

yes. thanks for you help!

@Rockingrajat
Copy link

Rockingrajat commented Oct 27, 2021 •
edited

Screenshot from 2021-10-27 19-52-02

@tehkapa @DrewGregory is this fine? I've changed the ReportIncident.tsx to read the user name and removed 'some user' placeholder from useReportIncident.tsx

Rockingrajat added a commit to Rockingrajat/hospitalrun-frontend that referenced this issue Oct 27, 2021
@Rockingrajat
fixed the reportBy issue HospitalRun#2539
b2fd7c1
This was referenced Oct 27, 2021
Closed
Open
@Rockingrajat
Copy link

@tehkapa I've created a PR can you please check?

@ykabusalah
Copy link

As this issue is currently open, has it been fixed? Or could I claim this issue and attempt it myself?

@farrukhrashid1997
Copy link

Is this issue still open? I'd like to contribute.

@Rockingrajat Rockingrajat removed their assignment Jun 20, 2022
@xhefribala
Copy link

@tehkapa Could you please assign me to this issue?

This was referenced Sep 9, 2022
Merged
Closed
@xhefribala xhefribala linked a pull request Sep 19, 2022 that will close this issue
Draft
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@xhefribala xhefribala

Labels
馃悰bug issue/pull request that documents/fixes a bug good first issue indicates an issue is good for a first time contributor in progress indicates that issue/pull request is currently being worked on incidents
Projects
Version 2.0
  
To do
Milestone
v2.0