slidewiki-platform
slidewiki-platform copied to clipboard
Swik 907 delete deck and transfer ownership
I added a new button to deck edit and a workflow for delete/remove/transfer ownership of a deck. Not all deck routes are working yet. Please verify that the workflow is correct. Do you have suggestions for visual improve the user list when transfering ownership?
Missing:
- when being on a subdeck - differences?
- Intl texts
TODO is also Kostis comment:
Not sure if I have it in the flow, but all those options should be available only to the owner/creator of the subdeck or deck we want to delete. That is, for a simple editor of the deck in question:
- the deck edit panel SHOULD NOT have the delete deck button
- the remove subdeck button SHOULD NOT show the modal to choose from, action should be as it is now.
I have opened PR https://github.com/slidewiki/deck-service/pull/149 ( not ready yet of course ), and also filled in some missing calls in this branch as well to better test.
- we should have an "are you sure" ? message before deleting a deck when none of the cases where we prompt with info or for more actions to the user.
- instead of the home page, after deleting a deck we should redirect to the user's "my decks"
- my suggestion for the delete subdeck modal
was that we show this when user clicks on "remove subdeck" icon to to the top right of the toolbar:
, in case the only parent (usage) of this deck was the parent currently displayed as well, and from which the user wants to remove the subdeck.
- likewise, there should not exist a "delete deck" button under a subdeck ( regardless if it was loaded as such: a subdeck is a deck that has usage )
- some modals don't have "cancel":
- in the previous modal, I hit
esc
and the transfer modal appeared, I think it's a bug, or something Kurt left TODO in case we wanted to show the tranfer panel. For subdecks I think we should keep it simple, either remove from tree (like before) or delete altogether. - in other cases the modal shows fine, it also lists all current members of all groups it seems, even greater :+1:
For deleting a subdeck, you should use actions/decktree/deleteTreeNode
that uses the API, /decktree/node/delete
. If the user has selected to also delete the node, please send in addition to the selector a property purge
equal to true
. The service will also delete the node if it's a deck and if the restrictions we are discussing apply.
What are the possible use cases?
- deletion of deck with no usage 1.1 no editors - just deletion 1.2 transfer ownership instead of delete the whole deck
- deletion/removable of subdeck 2.1 viewing the subdeck as subdeck in the parent deck 2.1.1 delete subdeck as child everywhere and deck completely 2.1.2 remove subdeck from current parent parent 2.1.3 remove subdeck from all parent parents 2.2 viewing subdeck as own deck 2.2.1 delete subdeck as child everywhere and deck completely 2.2.2 remove subdeck from all parent decks
Thus the delete button in edit view for root decks and subdecks opened as own decks. When viewing a subdeck as child of its parent we have the trash icon button for deleting the node.
I'm confused with (2) use cases, let me rephrase them as I understand them:
- deletion/removal of subdeck 2.1 viewing the subdeck as subdeck in the parent deck 2.1.1 remove subdeck from current tree AND delete, as long as usage is limited to current parent 2.1.2 remove subdeck from current tree (exactly what we do now) 2.1.3 ~remove subdeck from all parent parents~if a subdeck is subdeck to other parents, do not offer option to delete. 2.2 viewing subdeck as own deck => do not offer option to delete at all (like we have now) 2.2.1 ~delete subdeck as child everywhere and deck completely~ 2.2.2 ~remove subdeck from all parent decks~
If 2.1.1 could have more than one usage entry, then 2.1.3 is valid because usage and parents is the same thing?
2.1.1 means there's only one usage: the parent currently in view 2.1.3 if there are more that one parents in usage, we don't support deleting, ONLY removing from current parent -> it will still be available in some other deck tree
@abijames I updated the branch with the changes we discussed during amsterdam plenary, so in order to delete a subdeck user simply removes it, then they can delete it after loading it from "My decks". So, no option is given whether to simply remove or also delete. I've also moved the delete button to the right so it's more separated from save/cancel buttons.
Also, provided a fix for the tranfer ownership dialog not closing sometimes. But I would like @TBoonX to look at that as well, as I commented out some code I didn't know much about.
I tested this on athena server and have feedback to improve the UI. Functionally it all worked well except for 2 issues:
- I transferred ownership of a deck. A fork of the deck appeared in the search results and still listed me as the original author. This caused 2 problems 1.1 when you clicked on the original deck the author changed to the new owner 1.2 when you click on the link for the original author in the forked deck you get taken to a My Decks list which no longer contains the original deck
- when I transferred a published deck, it become unlisted - is this correct?
Suggested UI changes and accessibility issues:
- ~On deck edit the Delete button should be named "Delete Deck" and it could also have the trash can icon~
- ~When attempting to delete a deck with a sub-deck, change the message to: "You can only delete decks with no sub-decks. Please remove the sub-decks first and try again."~
- Message when deleting a deck 3.1 ~Does this need to include the deck ID? We don't show this to the user at any other time~ 3.2 can you make the buttons Green for OK 3.3 ~Change message on this dialog to: "Do you want to delete the deck "[deck name]"? This action can not be reverted."~
- ~After deleting a deck a message appears say "Success" with a confirmation button. Change the button to "OK".~
Transfer ownership:
- You can't select another owner with the keyboard. Have a look at attach deck modal for a keyboard accessible implementation. At the moment when you tab you go to the link for the authors user name.
- There needs to be a more visible indication of which editor you've selected. Very unclear when there is more than editor to choose.
- ~Change the text in the message to: "Other users have rights to edit this deck. You can transfer the deck to another editor or delete the deck for everyone."~
- ~Change the text in the buttons to [Transfer deck] [Delete deck] [Cancel].~
- If you select delete deck then you should see the warning modal as you do when you delete a deck without other editors.
Tested on Athena server. Have striked out my comments that have been addressed.
@abijames regarding the issues you've had:
I transferred ownership of a deck. A fork of the deck appeared in the search results and still listed me as the original author.
Your transferring does not affect a fork. If that fork was owned by you, it will remain, unless you transfer ownership of that as well.
1.1 when you clicked on the original deck the author changed to the new owner
This is to be the expected, why is it an issue?
1.2 when you click on the link for the original author in the forked deck you get taken to a My Decks list which no longer contains the original deck
Also correct, transferring ownership removes the deck from the original author's "My Decks". Although I'm not sure where that link is, is it in the search results?
- when I transferred a published deck, it become unlisted - is this correct?
This is intended, yes, so as to allow the new deck owner to review this deck before letting this get published as their own.
@kprist, thanks for comments. Re (1) my corner is the lack of consistency. Opening the deck from search results to the landing page then the owner of the deck changes. It would be better if the new owner was shown in the search results instead
I see, if it's the same deck and their metadata change, that is a bug, I think the search indexer indeed fails to update the deck owner.
@abijames I've resolved most of your comments in the athena deployment. Only thing missing should be
If you select delete deck then you should see the warning modal as you do when you delete a deck without other editors.
@abijames I also verified tranferred owners are properly updated with the search service. There was an issue with some updates in deck service master not in this branch that prevented proper search index updates. Please re-check.
Keep in mind that after transferring a deck, it should be removed from search index (as it is also unlisted). The new owner needs to re-publish it it order to be discoverable.