Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polls 5.4.0-beta5 released (12-10-2023) #3107

Closed
github-actions bot opened this issue Oct 12, 2023 · 11 comments
Closed

Polls 5.4.0-beta5 released (12-10-2023) #3107

github-actions bot opened this issue Oct 12, 2023 · 11 comments

Comments

@github-actions
Copy link

Changelog for the upcomming release (preview)

Fixes

  • Fixed granting admin rights to shares
  • Fixed a bug, where exports were prevented
  • Fixed a visually bug when using Nextcloud's Dark Mode

New

  • Reveal hidden voters if hidden in case of performance concerns
  • Support better readability of vote page
  • Added revoking of shares
  • Shares can now be revoked which works as a read only share mechanism. Revoked shares can still enter the poll, but all interaction (voting and commenting) is disabled.
  • Deletion of unsent shares have no more a redo timer
  • Deletion of revoked shares deletes the users votes as well

Changes

  • Improved username check for public polls with a large number of groups in the backend

Downloads

Overview and changelog: https://github.com/nextcloud/polls/releases/tag/v5.4.0-beta5
Download ZIP: https://github.com/nextcloud/polls/releases/download/v5.4.0-beta5/polls-5.4.0-beta5.zip
Download TAR.GZ: https://github.com/nextcloud/polls/releases/download/v5.4.0-beta5/polls-5.4.0-beta5.tar.gz

@dartcafe dartcafe pinned this issue Oct 12, 2023
@dartcafe
Copy link
Collaborator

Since revoking a share first came with another intention and it turned into a function to switch shares into a read-only mode.

Consequences of revoking a share

  • The sharee

    • can't vote anymore,
    • can't comment anymore,
    • can't see the comments anymore,
    • has no information, that he is in read only mode
  • The poll owner

    • may not understand that revoking a share means, the share is now a read-only one

Possible solutions:

  • explicit information for the sharee, that he is in a read only mode
  • change term "revoking" into something like "read-only" i.e.
    • "revoke share" -> "set to read only"
    • "re-revoke share" -> "allow normal access"
    • "Revoked Shares" -> "Read only shares"

Anyone with suggestions?
@PhrozenByte

@PhrozenByte
Copy link
Contributor

I feel like that your suggested solutions shouldn't be an "either or", but both.

I agree that "revoking" might be a bit confusing, at first glance I'd assume that "revoking" is synonymous to "deleting" a share, from what I'd rather expect that the sharee looses full access. Thus I think that renaming this to "read only" or similar is best. Some alternative names I can think of are "enabled/disabled share", "limited share", or straight "close poll for participant" - but I can't really claim that I feel like one or the another is truly best.

Independently, some explicit information for the sharee that the poll wasn't closed yet, but just that his share has been limited, is desirable no matter what.

@PhrozenByte
Copy link
Contributor

PhrozenByte commented Oct 13, 2023

Another naming idea I just had is "active/inactive share" (to be more precise: "active/inactive" to describe the state, "enable/disable" to describe the action). I still don't feel like that one alternative is truly better than the other, but my gut votes for "active/inactive share".

This also made me thinking whether we could also make public shares active/inactive - and came to a "Why not?" conclusion. For a public link to become "inactive" the only (visible) thing we need to do is that no name/email modal pops up - the public link itself already is read-only.

Besides public shares, are there any other share types that don't support revocation / access limitation yet?

Another, but strongly related suggestion: When the poll is closed all shares should be listed as "inactive", again strengthening the connection to closed polls (I don't mean that they should be set inactive/revoked in the database, they should just be shown in the list like if they were).

Edit: Just noticed that email shares (or any single user vote?) without a vote can't be revoked. Why is that?

@dartcafe
Copy link
Collaborator

@PhrozenByte Thanks for your sparrings.

I feel like that your suggested solutions shouldn't be an "either or", but both.

They were not meant as alternatives. 😆

Just noticed that email shares (or any single user vote?) without a vote can't be revoked. Why is that?

The idea was, that unvoted shares should be deleted right away instead of revoking them. But the effort for exxplaining this idea will cost more than the possibility to revoke shares whatever state they have could cause harm. To be honest: Which harm cane be made???

Besides public shares, are there any other share types that don't support revocation / access limitation yet?

The restrictions intentionally applied only to public shares. But I was wondering if this restriction is necessary in any way. Spoiler: no.

Conclusions:

  • The terms have been changed from revoked to locked. It is a simple and precise term.
    disabled clashes with disabled states of components in the code and would cause more work.
    activated would possibly need more information, because there are no actions and could be misinterpreted.
  • The database schema and the usage of the term locked is used consistently in the code instead of revoked
  • Locking/unlocking is now possible for all shares, no matter if it is a public share or if the user has voted or not
  • The lock action has been moved to the context menu of the share and deletion of shares has returned to all shares.
  • Users of locked shares get an information banner on the top of the poll page, that tey just have read access.

Revoking of a share instead of deleting will come back in another pr to replace the deletion counter, which is ugly IMHO.

lock share unlock share unlocked shares list user information
grafik grafik grafik grafik

@PhrozenByte
Copy link
Contributor

Ah, "locked share", why didn't I think of that? So obvious 🙈 Yeah, that's the best one 👍

I truly don't have anything to add, this sounds just perfect. Great work, thank you! 👍

@PhrozenByte
Copy link
Contributor

Just coincidentally noticed that the term "revoke" was still used at some place, so I quickly searched through the code and there are some more usages left:

  • In CHANGELOG.md
    - Added revoking of shares
  • API endpoint naming (do they even exist? because there are no matching methods in ShareApiController)

    polls/appinfo/routes.php

    Lines 155 to 156 in c651f1c

    ['name' => 'share_api#revoke', 'url' => '/api/v1.0/share/revoke/{token}', 'verb' => 'PUT'],
    ['name' => 'share_api#re_revoke', 'url' => '/api/v1.0/share/rerevoke/{token}', 'verb' => 'PUT'],
  • Some JavaScript error messages
    showError(t('polls', 'Error deleting or revoking share for user {displayName}', { displayName: share.displayName }))
    console.error('Error deleting or revoking share', { share }, e.response)
    showError(t('polls', 'Error deleting or revoking share for user {displayName}', { displayName: share.displayName }))
    console.error('Error deleting or revoking share', { share }, e.response)
    console.error('Error revoking share', { error: e.response }, { payload })
    console.error('Error re-revoking share', { error: e.response }, { payload })

@dartcafe
Copy link
Collaborator

I think you looked into an outdated commit. Check the current master.
i.e.

polls/appinfo/routes.php

Lines 101 to 103 in c651f1c

['name' => 'share#lock', 'url' => '/share/{token}/lock', 'verb' => 'PUT'],
['name' => 'share#unlock', 'url' => '/share/{token}/unlock', 'verb' => 'PUT'],

@dartcafe
Copy link
Collaborator

dartcafe commented Oct 15, 2023

Ah. f**k. no I forgot the api endpoints. And searching for revoke does not find revoking
I will deliver it later

@dartcafe
Copy link
Collaborator

@PhrozenByte Honestly a million thanks for your effort and review. Feeling lonely here sometimes. 😉 Beta6 is going to be build and out in minutes.

@dartcafe dartcafe unpinned this issue Oct 15, 2023
@PhrozenByte
Copy link
Contributor

You're very welcome! I know the feeling sometimes 🙈 You're doing an amazing job here! ❤️ Thanks for beta 6 and rc 1, all work like a charm. As promised I gave some feedback about the new feature in #3099 (comment).

Copy link
Author

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants