search

codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
28 stars 25 forks source link

Issue 377/private profile info and Issue 387 fixing profile bug #379

Closed leekahung closed 1 year ago

leekahung commented 1 year ago

Create Private Profile Information on User's pod

This PR:

1. Generates a new Profile container (in /PASS) with a privateProfile.ttl (this container is separate from the public /profile container) 2. A new ACL file will be set for Profile; a follow-up PR will be created to account for permission adjustments 3. Including a single new field for the profile card using information from privateProfile.ttl as proof-of-concept 4. Utilizing schema for date of birth (DOB) from hmis-interop for private profile (https://github.com/hmis-interop/logical-model/blob/FY2022-v1.0.0/src/logical-model.n3.owl#DOB) 5. Included fixes for #387 where client, clientProfile, and setClient from Profile, ProfileComponent, DocumentListContext, etc. needs to be updated to contact, contactProfile, and setContact, respectively

Clip of Profile update with private profile:

https://github.com/codeforpdx/PASS/assets/14917816/60cfa735-b45c-40bc-9372-211b9ec4f0ed

Clip of bug fix for contacts:

https://github.com/codeforpdx/PASS/assets/14917816/e4a1e19f-cb91-402c-808e-9d6fb3b135d7

The format used for the HMIS schema follows what is instructed in the model overview documentation (https://github.com/hmis-interop/logical-model/blob/FY2022-latest/doc/HMIS-logical-model-overview.pdf, page 4)

The files this PR effects:

Components

Tests

Other Files

Future Steps/PRs Needed to Finish This Work (optional):

leekahung commented 1 year ago

Hey @timbot1789 and @xscottxbrownx, I've recently incorporated the new changes from the recent #362 merger with React Query and Contacts and included fixes to other locations that utilizes Clients/Contacts on top of the feature proposed here.

Users should be able to view other user's profile again after merging these fixes

leekahung commented 1 year ago

Checked file changes and look good to me, BUT I'd prefer more eyes on the rdf predicates, model-helpers, and the tests

I'm still only seeing my profile when clicking on contact name, but I am testing contacts that don't actually exist - so that's most likely the problem. Would just like confirmation of that.

I typed in random stuff when adding a contact to my list.

UPDATE:

Tested ⬆️ - these same "contacts" send me to that contact page in issue-358 branch. So I believe still something wrong here.

As for the contact links sending you to the wrong profile, this is due to the recent merge with React Query didn't have fully incorporated the changes for all client/clients/clientProfile to contact/contacts/contactProfile, see issue #387. This branch has been fixed to send you to the proper profile if the webId exists. (see clip below)

https://github.com/codeforpdx/PASS/assets/14917816/07f4207d-14af-40e8-b958-6e859d5315e5

At the moment, if you created a link who's webId does not exist, it'll send you back to your profile with that incorrect webId as you mentioned. That is certainly a bug, but I think with the new registration flow Tim's making in PR #378 and potentially a sign-in flow in future PRs for users to link with case workers, I don't think it's too urgent of an issue. With those updates, I believe case workers don't have to manually add the webIds themselves and clients would simply need to sign up/register with a case worker and their webIds would be automatically included in the list? At least that's what I think?

xscottxbrownx commented 1 year ago

At the moment, if you created a link who's webId does not exist, it'll send you back to your profile with that incorrect webId as you mentioned. That is certainly a bug, but I think with the new registration flow Tim's making in PR #378 and potentially a sign-in flow in future PRs for users to link with case workers, I don't think it's too urgent of an issue. With those updates, I believe case workers don't have to manually add the webIds themselves and clients would simply need to sign up/register with a case worker and their webIds would be automatically included in the list? At least that's what I think?

Okay, just so I'm clear on the direction of the app again...

Contacts page will only be visible to organizations (maybe) and caseworkers, not clients at all correct?

AND we are not interested in giving the ability for clients to "connect with"/create contact with other clients correct? So no seeing/sharing between random friends, only clients and caseworkers/orgs?

leekahung commented 1 year ago

AND we are not interested in giving the ability for clients to "connect with"/create contact with other clients correct? So no seeing/sharing between random friends, only clients and caseworkers/orgs?

Just for MVP I think.

I do think it's a good idea to be able to share things to other people if users choose to. So making the page as contacts was a good one. Makes it a kind of address book for users.

leekahung commented 1 year ago

Contacts page will only be visible to organizations (maybe) and caseworkers, not clients at all correct?

From what I understand from the PR for switching to contacts instead of clients, it's meant to be universal? (See Issue #351) Except case workers and orgs have the ability to gain permission to documents and private profile information.

xscottxbrownx commented 1 year ago

AND we are not interested in giving the ability for clients to "connect with"/create contact with other clients correct? So no seeing/sharing between random friends, only clients and caseworkers/orgs?

Just for MVP I think.

I do think it's a good idea to be able to share things to other people if users choose to. So making the page as contacts was a good one. Makes it a kind of address book for users.

Ok, so does that work now - it's just not in my case because I made people up that don't exist?

leekahung commented 1 year ago

Ok, so does that work now - it's just not in my case because I made people up that don't exist?

As shown from the clip I've posted in an earlier response, if the webId being used is something random and does not exist, the current behavior is to go to profile with the made up webId that leads to an empty profile card on the pod and the profile information shown on PASS will be based on your profile since the state for contact would be set to null in this case for the profile link in Contacts because profile information does not exist for this person.

xscottxbrownx commented 1 year ago

Ok, so does that work now - it's just not in my case because I made people up that don't exist?

As shown from the clip I've posted in an earlier response, if the webId being used is something random and does not exist, the current behavior is to go to profile with the made up webId that leads to an empty profile card on the pod and the profile information shown on PASS will be based on your profile since the state for contact would be set to null in this case for the profile link in Contacts because profile information does not exist for this person.

ok, that's confusing... no user will expect to click another user and see their own info.

So yeah, I guess we can chalk it up as a bug. I guess the best thing would to not allow a user to create a contact that doesn't exist. Next best option would be to have some sort of "user does not exist" show and then redirect back to contacts page?

Sounds like a separate PR/Issue - got it. So now I agree with this PR and would approve if I knew more about the model-helpers and the test. So, I'll leave that for @timbot1789 to take a look at.

The rest looks good to me!

leekahung commented 1 year ago

ok, that's confusing... no user will expect to click another user and see their own info.

Yeah, think it's mentioned in one of the newer issues too (#386)

So yeah, I guess we can chalk it up as a bug. I guess the best thing would to not allow a user to create a contact that doesn't exist. Next best option would be to have some sort of "user does not exist" show and then redirect back to contacts page?

Think that would be a good solution.

leekahung commented 1 year ago

Hey @timbot1789. This should be ready for another pass. Let me know if you think it's good to merge like @xscottxbrownx.