571/edit contacts

[JaeGif]

This PR:

Resolves #571

Can now edit contacts clicking the icon, and changing just their given or last names. OIDC and Username are inferred from the PodUrl.

• Adds editing state to AddContactModal
• Selectively displays oidc and username inputs if editing
• Added parsePodUrl helper function to extract the oidc and the username from the existing podurl when editing a contact

Extension of the work done at #613

Future work: parsePodUrl can probably be improved if the need arises, the function right now assumes all oidc's will be standard url's which I hope is the case.

[JaeGif]

Failing tests don't seem to be related to this PR.

[leekahung]

Failing tests don't seem to be related to this PR.

Yeah, the tests are from an earlier PR that needs to be fixed.

[JaeGif]

I'm unable to edit the contact as the Edit Contact button stays disabled. Requesting changes as a safety measure to ensure it does indeed function as intended.

The issue says the web ID should be editable as well, though that is trickier.

What does the test user you're using look like? Are you using a different provider than localhost potentially?

[andycwilliams]

@JaeGif I'm using solidcommunity for both the account I'm logged in with as well as the contacts I try to edit

[JaeGif]

@JaeGif I'm using solidcommunity for both the account I'm logged in with as well as the contacts I try to edit

I see. It looks like the different provider formats the podURL differently. I found the schema for the different podURL possibilities and will update the parsing function.

[leekahung]

@JaeGif I'm using solidcommunity for both the account I'm logged in with as well as the contacts I try to edit

I see. It looks like the different provider formats the podURL differently. I found the schema for the different podURL possibilities and will update the parsing function.

Yeah, the format for webIds on solidcommunity.net uses a https://<username>.<provider>/ instead of the usual https://<provider>/<username>/ format from community solid server

[JaeGif]

I'm unable to edit the contact as the Edit Contact button stays disabled. Requesting changes as a safety measure to ensure it does indeed function as intended. The issue says the web ID should be editable as well, though that is trickier.

What does the test user you're using look like? Are you using a different provider than localhost potentially?

I believe I've fixed the issue you had.

As for the WebID, I'm not convinced that it's something that should be "editable" as my understanding is that it's a unique identifier of a user.

If we are sure we do want this though, I believe I can add this functionality quickly.

[andycwilliams]

@JaeGif Will take another look soon. Have you tested other pod providers, like opencommons?

The issue states to be able to edit the Web Id too. If we don't think parts of an issue are important, it would be best to mention that and open for discussion beforehand.

[leekahung]

The issue states to be able to edit the Web Id too. If we don't think parts of an issue are important, it would be best to mention that and open for discussion beforehand.

There's additional context to that issue. As stated since webIds is the identifier for the object, updating webId would likely require deleting the old entry and adding a new one anyways. The solution here is fine.

If users want to, they could simply make a new contact with the new webId. It's like adding a person's gmail account and their yahoo account onto contacts.

[andycwilliams]

It can just be mentioned in the PR that some part intentionally wasn't added because you don't think it's actually needed. It's good to have more communication and ensure that nothing appears to be an oversight or anything.

[leekahung]

Hey @JaeGif @andycwilliams, would like to check up with you guys. What's the current status of this PR?

[andycwilliams]

Hey @JaeGif @andycwilliams, would like to check up with you guys. What's the current status of this PR?

Once it's possible to update the contact regardless of OIDC Provider I'm ready to approve

[JaeGif]

Hey @JaeGif @andycwilliams, would like to check up with you guys. What's the current status of this PR?

Once it's possible to update the contact regardless of OIDC Provider I'm ready to approve

I've checked with all the current oidc providers, and parsePodUrl successfully parsed all providers on the VITE_OIDC_WEBIDS list.

I'm not particularly good at tests so I'm uncertain how to best write a unit test for this as the parsePodUrl function is inside of the AddContactModal. Everything I can think of needs a separate test for each OIDC which doesn't feel like the best way to do it.

I've tested the function by separating parsePodUrl to it's own file locally, and iterating across VITE_OIDC_WEBIDS. All oidc's and usernames are parsed out correctly in this way.

[andycwilliams]

I've checked with all the current oidc providers, and parsePodUrl successfully parsed all providers on the VITE_OIDC_WEBIDS list.

I'm not particularly good at tests so I'm uncertain how to best write a unit test for this as the parsePodUrl function is inside of the AddContactModal. Everything I can think of needs a separate test for each OIDC which doesn't feel like the best way to do it.

I've tested the function by separating parsePodUrl to it's own file locally, and iterating across VITE_OIDC_WEBIDS. All oidc's and usernames are parsed out correctly in this way.

If it works for all providers then this is pretty much ready. The only remaining thing is that the development branch has since gotten new merges, so this will need updating. Shouldn't be too much though, just adding the same functionality as a menu item for the mobile version of the contacts table.

As for tests, yes normally we want them completed with each PR. However, I'm making an exception for MVP by creating an issue (#662) to address all failing tests. So if you can write them, or just as much as you can, then that's great, but otherwise it ought to be covered by that.

[JaeGif]

I've checked with all the current oidc providers, and parsePodUrl successfully parsed all providers on the VITE_OIDC_WEBIDS list. I'm not particularly good at tests so I'm uncertain how to best write a unit test for this as the parsePodUrl function is inside of the AddContactModal. Everything I can think of needs a separate test for each OIDC which doesn't feel like the best way to do it. I've tested the function by separating parsePodUrl to it's own file locally, and iterating across VITE_OIDC_WEBIDS. All oidc's and usernames are parsed out correctly in this way.

If it works for all providers then this is pretty much ready. The only remaining thing is that the development branch has since gotten new merges, so this will need updating. Shouldn't be too much though, just adding the same functionality as a menu item for the mobile version of the contacts table.

As for tests, yes normally we want them completed with each PR. However, I'm making an exception for MVP by creating an issue (#662) to address all failing tests. So if you can write them, or just as much as you can, then that's great, but otherwise it ought to be covered by that.

Sounds good. Just reconciled the branches. Some slight changes had to be made to fit the new layout, made the minor changes and tested in every way I can think of and got it working.