ucd-library / aggie-experts

Publicly reported feedback and issues for Aggie Experts
https://ucd-library.github.io/aggie-experts/
MIT License
1 stars 2 forks source link

Qh expert visibility #358

Closed qjhart closed 2 months ago

qjhart commented 2 months ago

This branch adds support for an expert level visibility flag. This is done by adding adding is-visible:true|false at the document level for all experts. All search and browse functions use this query. It's also included in the sanitization step.

The premise for this is that experts can be in the system, but not be visible. If an admin or the expert themselves do to their profile page, then they have the capability to edit their pages, and make themselves visible. There are potentially two flags there, one affecting visibility, and one removing the expert from the system entirely.

However, in the current framework, we don't expect anyone not-visible to be included in the system, and only the removal button will be available. This removes the expert entirely from the system.

We finished the visibility component, because we didn't want to leave it out or half finished, and because the components in the elements-client library and the expert model.js both now have better support for updates to a user (like their themes or media tags)

Removal

I have verified that the current fcrepo implementation does remove all children from the system on a delete.

     dc exec fcrepo  bash  -c 'find /usr/local/tomcat/fcrepo-home/data/ocfl-root -name 0=ocfl_object_1.1 | wc -l '
    # 102
     host=http://localhost
     e=66356b7eec24c51f01e757af2b27ebb8
     http DELETE $host/fcrepo/rest/expert/$e/fcr:tombstone Authorization:"Bearer $jwt"
     dc exec fcrepo  bash  -c 'find /usr/local/tomcat/fcrepo-home/data/ocfl-root -name 0=ocfl_object_1.1 | wc -l '
    # 63
UcDust commented 2 months ago

@qjhart

No rush, but question on the browse api. I think in aggie-experts/services/base-service/models/base/model.js in the msearch() function, an es query is made for the experts to build the browse A-Z endpoint calls.

I noticed when an expert has is-visible : false that they still are being returned in that endpoint, so they show up on the browse page.

Should we filter them out somehow in the es query, or should it be filtered out elsewhere?

qjhart commented 2 months ago

@qjhart

No rush, but question on the browse api. I think in aggie-experts/services/base-service/models/base/model.js in the msearch() function, an es query is made for the experts to build the browse A-Z endpoint calls.

I noticed when an expert has is-visible : false that they still are being returned in that endpoint, so they show up on the browse page.

Should we filter them out somehow in the es query, or should it be filtered out elsewhere?

@UcDust Addressed in 1b75072

qjhart commented 2 months ago

@UcDust right now if a user is in the system, but not visible, I return a 204. Alternatively, I could return a 404, or even a 200 w/ content {is-visible:false} I wonder what the best solution is.

UcDust commented 2 months ago

@qjhart Would it be possible to return a 200 with the normal content if the user is an admin, and return a 404 if the user isn't an admin?

I think in that case the client code wouldn't need to be updated. But also it seems like a better solution imo; the client shouldn't have an indication that the expert exists if they're hidden and the user isn't an admin. A 404 would make sense in that situation.

qjhart commented 2 months ago

@UcDust I want to test the harvest on this b4 committing, which is why we are waiting on oapolicy

Vensberg commented 2 months ago

Tests: Make user invisible in sandbox.

Delete user

@UcDust I have to refresh the page for the invisibility icon to activate. There is an error message that flashes on the screen when I refresh, but goes away too fast, so I can't read it.

In the message for delete, where it says "Expert will be removed from Aggie Experts. CDL privacy will be set to private", the setting is "internal" not private. I got an error message for delete, but after refreshing the page, the delete has worked in that the user is no longer in sandbox. However, @qjhart , after over 10 min your QA profile is still Public.

Vensberg commented 2 months ago

Regarding the error with deletion, it should be changed, so that the user is not removed from AE. If I encounter the problem, I will make them invisible, and manually change the visibility in Elements, so that they don't come in with the next data pull.

qjhart commented 2 months ago

Regarding the error with deletion, it should be changed, so that the user is not removed from AE. If I encounter the problem, I will make them invisible, and manually change the visibility in Elements, so that they don't come in with the next data pull.

@Vensberg, how about we keep both buttons in that case. The visibility will not remove the user, and will not change Elements, this means if you don't change it they might come back when revisited, but a deletion Will remove them, an will forward the change to CDL?

Vensberg commented 2 months ago

@qjhart This is a good solution.

qjhart commented 2 months ago

I added change to the visibility in #374