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

feat(neon_framework): Implement deleting accounts on the server #1670

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

provokateurin
Copy link
Member

Closes #732
We can not directly use the endpoint because of #1492, so I opted for implementing it like the mobile clients which just open the settings page in the browser. For that reason I also patched the spec to remove the endpoint as it is not possible to use it (yet).

@Leptopoda
Copy link
Member

I think another action in the appbar is confusing for users.
I think we should steal the desing of the official client nextcloud/android#12015.

A dialog with two options.

@tcitworld
Copy link
Member

Nice !
Also, the UI could benefit from the information exposed by the capacities:

  • the delay field saying whether the user data will be deleted straight away or after a while, which allows the accounts to be restored
  • the details field saying why the account can't be deleted if applicable

@provokateurin
Copy link
Member Author

The details field is already used, but the delay isn't yet. I'll rewrite the UI to use the dialog and then those can also be displayed in a nice way.

@Leptopoda
Copy link
Member

@provokateurin maybe don't make the dialogue adaptive.

The RadioListTile will require a material ancestor and even if we wrap it in a material widget it'll look out of place on iOS anyways.

@provokateurin
Copy link
Member Author

The CI failure is due to nextcloud/server#43877 and I hope I can get it into 28.0.3, otherwise we can just manually patch it for the time being (or update to a mid-release 28 snapshot).

@provokateurin provokateurin marked this pull request as draft February 27, 2024 17:20
@provokateurin provokateurin force-pushed the feat/nextcloud/drop_account branch from fd73366 to 2f546b6 Compare February 28, 2024 10:01
@provokateurin provokateurin changed the base branch from main to fix/nextcloud/provisioning_api-app-info February 28, 2024 10:01
@provokateurin provokateurin marked this pull request as ready for review February 28, 2024 10:01
@provokateurin
Copy link
Member Author

In case the server doesn't have the drop_account app the old dialog will be shown. I think it is pretty ugly to provide a radio that only has one option.
In case the user can't delete the account (e.g. last admin user of the server) the radio will be shown but the option is disabled. It now also tells you after what time the account will be deleted on the server (in addition to the reason why the account can't be deleted).

Base automatically changed from fix/nextcloud/provisioning_api-app-info to main February 28, 2024 12:17
@Leptopoda
Copy link
Member

So I'll need to manually install the app into the docker container to test it?
Looks like it isn't on the store (or am i missing something).

@provokateurin
Copy link
Member Author

So I'll need to manually install the app into the docker container to test it?

Just rebuild the docker images and delete your volumes.

Looks like it isn't on the store (or am i missing something).

https://apps.nextcloud.com/apps/drop_account it is, but you can't search for app IDs in the store...

@Leptopoda
Copy link
Member

Ok code and UI looks good.
Please add the doc comments and if possible also a unit test ❤️

@provokateurin provokateurin force-pushed the feat/nextcloud/drop_account branch from 2f546b6 to 5025c0e Compare February 29, 2024 15:24
@provokateurin provokateurin force-pushed the feat/nextcloud/drop_account branch from 5025c0e to f3d9966 Compare February 29, 2024 17:02
@provokateurin
Copy link
Member Author

Slow CI + a lot of random failure 🥲

@provokateurin provokateurin merged commit fda3bfe into main Feb 29, 2024
8 checks passed
@provokateurin provokateurin deleted the feat/nextcloud/drop_account branch February 29, 2024 19:21
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.

Delete account
3 participants