the-collab-lab / tcl-75-smart-shopping-list

2 stars 2 forks source link

Nk gb share list #35

Closed granolagabrielle closed 2 months ago

granolagabrielle commented 2 months ago

Description

This code allows the user to share a list with another recipient by entering their email address. If the email address doesn't exist in the system, the user gets an error stating that the list was not shared.

Related Issue

Closes #6

Acceptance Criteria

[X] The ManageList view shows a form that allows the user to enter an email to invite an existing user to a list, in addition to the form that allows them to add items to that list. [X] The input that accepts the email has a semantic label element associated with it [X] The user can submit this form with both the mouse and the Enter key [X] If the other user exists, the user is alerted that the list was shared [X] If the other user does not exist, the user is shown an error message that explains the problem

Type of Changes

enhancement

Updates

Before

After

Screenshot 2024-08-27 at 10 31 04 AM

Screenshot 2024-08-27 at 10 31 15 AM

Screenshot 2024-08-27 at 10 30 49 AM

Testing Steps / QA Criteria

  1. Select list that you want to share from the home page
  2. Navigate to "manage-list"
  3. Enter user email that you want to share the list with
  4. Click "Invite user"

Also verified in Firebase that the lists were shared.

github-actions[bot] commented 2 months ago

Visit the preview URL for this PR (updated for commit a4e5071):

https://tcl-75-smart-shopping-list--pr35-nk-gb-share-list-phw4kfkf.web.app

(expires Sun, 08 Sep 2024 17:25:27 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 1f1fd53c369e1fa31e15c310aa075b4e8f4f8dde

kweeuhree commented 2 months ago

Looks good, I am testing locally on my device and I noticed something peculiar, when I am signed in and entering my own email address (the one I signed up with) I am seeing the alert in the screenshot below.

Could this be a possible edge case to implement here?

the shareList function returns in case the recipient is the owner, so the list will not be shared, but we can certainly add a more descriptive message as in why the list was not shared!

Update: added a fix!

ahsanatzapier commented 2 months ago

Review:

QA:

kweeuhree commented 2 months ago

Great work on the implementation here, team! Unfortunately, a breaking change has been introduced: trying to type something in the Item Name input displays the user [object Object]. Another observation I made was that if I select the first list in the Home page, inputting anything in the email field returns You cannot share the list with yourself. (GIF). Strangely, when I create a new list and try to share that one with myself, I see the message List was shared with recipient. (GIF)

for some reason the path of the first list does not include current user id, so basically the shareList function checks for the following condition and returns:

if (!listPath.includes(currentUserId)) {
      return 'match';
    }

when i console log, the path of the list doesnt match my id, so it returns. I have to wonder whether it has to do with the fact that first list was created via firebase and shared between all of us? I cannot reproduce the issue for any other list that I manually created with my userId, and it looks like firebase createList function requires a userId in order to assign it to 'owner' property. createList function:

export async function createList(userId, userEmail, listName) {
    const listDocRef = doc(db, userId, listName);

    await setDoc(listDocRef, {
        owner: userId,
    });
...
}