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

Allow users to create ereaders #3531

Merged
merged 14 commits into from
Oct 26, 2024

Conversation

laustindasauce
Copy link
Contributor

Allow users to create ereaders

This is an enhancement feature that should close issue #1982 when merged. The purpose is to allow self-serve of the ereader devices by users of an Audiobookshelf instance. Personally, I'd much rather pass on the handling of ereader devices to the users themselves. Since it's disabled by default, it won't have any effect on current instances and only those who would like to utilize the feature can now have the opportunity to.

What's Changed

Client

  • Created label for new permission with value 'Can Create Ereader'
  • Added toggle to Account modal for user update/creation
  • For non admin/root, added ereader table to account page above logout button
    • Only allows edit/delete options for ereaders solely accessible by the user
  • Created new user ereader modal for creating/updating ereaders at user level
    • Removes accessibility options from the form (only name & email can be changed)
    • Only passes ereader devices solely accessible by the user to modal
  • Toast for duplicate name error when found by server

Server

  • Added createEreader permission for users with default false except for admin / root
  • Created new API route - POST: /api/me/ereader-devices
  • New MeController method to handle the route
    • Handles check for duplicate names across ereader devices not owned by the user
    • Only allows update/deletion/creation of devices with the current user as the only accessible user

Screenshots

User account page with createEreader permission

Admin users page with new toggle

@nichwall
Copy link
Contributor

This is generally working well for me and the correct devices show up for the user with limited editing if they are not the sole user who can access it. I did find some problems using the following tests.

Test 1:

  • Log in as user who can edit e-reader devices
  • Create device
  • Observe new device is present
  • Navigate to home page
  • Navigate to user page
  • Observe device is missing
  • Refresh page (using F5), device is now visible

Test 2:

  • Create 2 devices. One is only accessible to a specific user, the other is available for all users
  • Log in as the specific user
  • Add a device
  • The second device (available to all users) is overwritten in the table
  • Logging back in to root user shows that the second device is missing and was replaced by the newly created device

@laustindasauce
Copy link
Contributor Author

laustindasauce commented Oct 18, 2024

I was able to reproduce both issues and actually found another while doing that. These issues happened because I last minute decided to remove shared ereaders from the payload so that if a user uses the API they can't be a bad actor/unintentionally update any shared ereaders.

  1. Issue with removing shared ereader(s) when creating a new ereader
    Resolved by fixing the logic in how server side retrieves 'otherDevices' with this change

  2. Issue with changes not being pushed properly (forced reload)
    Resolved by fixing the how the clientEmitter is called for socket changes with this change

  3. Issue with inability to delete user owned ereaders

  • Would throw duplicate name error since we were including the shared ereaders in the API call
    Resolved by only passing the user owned ereaders with this change

@advplyr
Copy link
Owner

advplyr commented Oct 26, 2024

Solid, thanks!

@advplyr advplyr merged commit ecc30b8 into advplyr:master Oct 26, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants