deposit: provide and check publish permission for visibility

[rekt-hard]

❤️ Thank you for your contribution!

Description

Please describe briefly your pull request.

depends on inveniosoftware/invenio-rdm-records#1768
closes #2732
Provide can_publish permission when rendering the deposit page. Also reordered the permission alphabetically.

Front end check is performed in: inveniosoftware/invenio-rdm-records#1768

With permission:

Without permission:

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

• I'm aware of the code of conduct.
• I've created logical separate commits and followed the commit message format.
• I've added relevant test cases.
• I've added relevant documentation.
• I've marked translation strings.
• I've identified the copyright holder(s) and updated copyright headers for touched files (>15 lines contributions).
• I've NOT included third-party code (copy/pasted source code or new dependencies).
  • If you have added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Frontend

• I've followed the CSS/JS and React guidelines.
• I've followed the web accessibility guidelines.
• I've followed the user interface guidelines.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

1. You license your contribution under the same terms as the current repository’s license.
2. You agree that you have the right to license your contribution under the current repository’s license.

[max-moser]

this seems to be the only new value, the rest is just reordered?

[rekt-hard]

Exactly. Reordered them alphabetically and added publish

[max-moser]

Since this is a quite minimal and obvious code change (but with large positive impact), I dare say LGTM

[fenekku]

I am perhaps iconoclastic here: we found that our users can be confused by disappearing buttons like these. What about disabling the button and having a hover-over that explains why instead of removing the button completely?

I'll defer to any 3rd-party that chimes in though :)

[max-moser]

good point, perhaps the button should simply be "greyed out" with an on-hover tooltip.
WDYT @rekt-hard ?

[Samk13]

good point, perhaps the button should simply be "greyed out" with an on-hover tooltip. WDYT @rekt-hard ?

I agree with @max-moser, I did something similar in this PR which hasn't been merged yet. Here's a screen record:
inveniosoftware/invenio-rdm-records#1283 (comment)

[rekt-hard]

@fenekku @max-moser @Samk13 thank you for the input - very valid reasons. Just trying to depict the current state in InvenioRDM.

tldr; Setting the button active/inactive makes sense. Help on choosing the correct help text is welcome.

For requests, only the actions a user can perform are shown. For a user, a permission to perform an action usually also does not change by updating the request in some way. So it does not depend on some user action (like missing a required field) whether or not the action can be performed.

For rdm records on the deposit page, there is the share and publish button. These are always shown since from the back end, a user does fulfill the permissions to perform these actions. However, there is no sense in clicking on these buttons if the record is in an invalid state since this would lead to an error any way. So the front end is performing some extra checks for usability reasons. As soon as the record is in a certain state, the buttons becomes active (through user actions).

It looks like there are two possibilities:

• A user is not allowed to perform an action from the back end perspective -> do not display the button
• A user is allowed to perform an action from the back end perspective but the front end has additional checks for usability -> display the button but set it active depending on some local state

In my opinion, this issue and PR are more related to the second situation. The publish button will not become active just by filling out some fields. Another other action, triggered by some other user or the system will have to happen before the user will even be allowed to perform the publish action.

HOWEVER

In the case of a request, a user usually has enough context to understand that only certain actions are available anyway for them.
In the case of a new record, there might be several reasons why a user is not allowed to publish which might be unknown to the user.

I guess we can all envision the frustration of people wondering where the publish button went off to. Especially if it is present for record A but not for record B. In order to prevent this, simply letting the UI decide the state of the button seems to be more appropriate here - given that there will be meaningful hints/help texts explaining why a button is not active (which is not the case now by the way).

Since the possibility to click the button might be front end or back end related, there has to be some logic involved and probably also some additional payload from the back end. Ideas on how to conditionally set the correct help texts are very welcome :)

[fenekku]

Thanks @rekt-hard for laying out all those great thoughts. I definitely think this is a really grey area and, if anything, I'm more 55% for disabling rather than removing now with your explanations 😄 . Here are some of my thoughts to continue framing the problem and potential solutions.

Before that, I think what was meant here

It looks like there are two possibilities:b

• A user is not allowed to perform an action from the back end perspective -> do not display the button
• A user is allowed to perform an action from the back end perspective but the front end has additional checks for usability -> display the button but set it active depending on some local state
  In my opinion, this issue and PR are more related to the second situation.

was "related to the first situation", right? As-in: in the triggering situation for this need, the backend was returning a falsey can_publish, so the code was made to ensure the button is not displayed in this case and that was desirable because the backend is more fundamentally authoritative: if it says something is not allowed, it's not allowed until the backend is updated. Is that right? This is perhaps minor, but just to see if I understood the explanations ;).

Back to more thoughts: I think this is exactly emblematic of machine (JSON data) APIs vs Human APIs (HTML): for machines it makes a lot of sense to me to have binary "this shows up" / "this doesn't" in JSON data APIs because there is no human perception / expectations / conceptualization; for humans we want to account for past experience, expectations, possibilities. And that human API is super grey, because, if I now realize correctly, in the triggering scenario for the PR, only administrators can ever publish. So not showing the button doesn't interfere with regular users since only admins ever knew there was such a button, right?

But in a "default" instance, users are exposed to the publish button and it's an exception when they can't. can_publish could be affected by permission sharing in particular. An instance-specific overridden custom button might be the most appropriate solution to provide the kind of UX appropriate to that original scenario. If we do think it would be more helpful in general to have the button react to can_publish then disabling with hover text as part of this module would be my suggestion.

Now about providing the "why" as a hover tip. On further thoughts, I don't think we can reflect the backend reasons easily: it may have to do with request state + permissions which have both deep reasons... It wasn't immediately obvious to me (probably was to others), but on further inspection it dawned on me 🤦 . The front-end reasons for disabled publishing are mostly "Missing fields" IIRC. So having _("You don't have the permission to publish") when can_publish is falsey and _("Fields are missing.") in the deeper code inside the button for frontend reasons would do it. For the _("Fields are missing."), I don't think it's necessary for this PR / we can make a ticket. I also think that trying to generalize now for other actions is too early/we don't have enough data/experience, so wouldn't bother with other scenarios than this one at hand.

Conclusion from my POV: 2 potential solutions: either an instance specific overridden button to get the UX desirable for that instance OR disable button with hover text just stating that can't do the action because of permission (not which permission in particular because too complex, so not much of a why 🤷 ).

Hope that helps!

[kpsherva]

I hope you don't mind if I jump in with some UX suggestions, even if it is late.
From our experience, hiding elements of the interface is confusing for the end user - we receive support tickets stating "My colleague sees the button and I am not, there is a bug"
In order to avoid the confusion, in all of the cases we disable the button instead of hiding it, and we are adding the tooltip explaining why the button is disabled.
Could you align the changes to follow this approach to make it consistent?

[rekt-hard]

@fenekku @kpsherva Thank you for the input - we are on the same page now and reached the same conclusion 😄
I will update the implementation accordingly

[rekt-hard]

@fenekku @kpsherva @max-moser @Samk13
Updated this PR to pass the permissions in the front end.
Also created this PR for the publish button component: inveniosoftware/invenio-rdm-records#1768

[fenekku]

@rekt-hard thanks for the work on this! If you need/want/can version / merge / release all the better or we can piggyback on other releases of these modules.

[rekt-hard]

@fenekku I do not have the permissions to do that. Would be great to get this into v12 though if possible :)

[max-moser]

i'd like to see this in v12 too, but i think in yesterday's telecon it was agreed to freeze non-critical bug fixes etc. for v12 for now – so maybe v12.1?
ping @utnapischtim

[fenekku]

What @max-moser said - defer to @utnapischtim + @kpsherva for this release's management. However I really think we should aim to have a 12.1, .2 ... series for v12 and have so with a faster turnaround. I'd put that there definitely. v11 was released in January of 2023. A discussion about pace/content/approach of releases would be warranted.

[github-actions]

This PR was automatically marked as stale.

[max-moser]

like its sibling PR, could you please rebase it? @rekt-hard
then i'll merge it