Sage-Bionetworks / sagebio-collaboration-portal

Collaboration Portal developed by Sage Bionetworks
1 stars 0 forks source link

Test user doesn't have edit/delete access to the discussion thread they created #567

Closed ychae closed 4 years ago

ychae commented 4 years ago

I think this is related to the general auth update but I wanted to track specifics where I could so I don't forget.

When I create a thread as test user, the ellipses drop-down doesn't show any options for me. However, when I log in as admin all the options are there. test-user-parent-thread

As test user I have copy link, edit, and delete access to the parent thread once I click into the thread. But I don't get those options to the sub-threads and the thread in the main list of discussions: auth-thread-non-admin

tschaffter commented 4 years ago

Specifications

Project

Similar approach for

ychae commented 4 years ago

The following functionalities were tested in General Discussion, Project Discussion, Tool Discussion, and Data Catalog Discussion:

General, Tool, and Data Catalog Discussion - Test user creates new discussion:

Project Discussion - Test user creates new discussion in a project that they created:

Test user creates new discussion in a project that they are invited to with READ/WRITE/ADMIN access:

Test user adds to an admin created discussion in a project that they are invited to with READ/WRITE/ADMIN access:

ychae commented 4 years ago

@tschaffter Thanks so much for all the updates to the discussions! Please see my comment above for results from testing -- long story short, all the expected behavior was working great!

The only odd thing I found was that when I deleted threads from a discussion, the participant icon list on the main page didn't update (see screenshot below):

discussion-icon-discrepency

To reproduce:

  1. Log in as admin user
  2. Go to any discussion (this example uses the General Discussion page)
  3. Create a new discussion
  4. Add in a couple of responses as admin (responses can also be added as test user)
  5. Go to the main list of general discussion: you should now see the number of replies and a few new user icons
  6. Go back into the discussion and delete all the responses
  7. Go back to the list of general discussion: the number of replies are gone but the number of user icons remains the same.
tschaffter commented 4 years ago

@ychae The discussion is made of two models, Thread and Message. The Thread has a property called contributors. Each time a Message is added to a Thread, contributors is updated to keep track of the last N users who posted a message. The reason I have added the contributors property is that it's more efficient to get this information quickly rather than pulling the last N messages and extract their authors. The drawback is that contributors is a duplicated piece of information and because there is not reference link between messages and contributors, removing a contributor at the time of removing a message is not trivial. Moreover, the discrepancy we see is because a delete message completely disappear. In the future I not do it that way but instead still show that someone posted a message but deleted it. Not deleting purely objects but instead "deactivate" them would also be more provenance friendly. For now I recommend not changing the implementation as someone did contribute at some point in the discussion, even if the contribution has been deleted since.

ychae commented 4 years ago

Ah that makes sense. Thanks for clarifying!