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

Add content action. #54

Merged
merged 6 commits into from
Oct 6, 2022
Merged

Add content action. #54

merged 6 commits into from
Oct 6, 2022

Conversation

ssinnott
Copy link
Contributor

@ssinnott ssinnott commented Sep 29, 2022

This adjusts the remove space user function to support content_action instead of remove_projects. The workflow here is going to change shortly and when you remove users from a space you will only be able to: leave their content in the space, archive their content (using the relatively new space content archive), or move the space to the trash.

Closes #53

Copy link
Collaborator

@mine-cetinkaya-rundel mine-cetinkaya-rundel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I don't think we have to change the argument names, I think it would be good to do so, to match how arguments are generally defined in R. And at a minimum, the docs should indicate these are character strings anyway (so have quotes around them(.

R/members.R Outdated Show resolved Hide resolved
R/members.R Outdated Show resolved Hide resolved
R/members.R Outdated Show resolved Hide resolved
R/members.R Outdated Show resolved Hide resolved
Sean Sinnott and others added 4 commits October 3, 2022 08:32
Co-authored-by: Mine Cetinkaya-Rundel <cetinkaya.mine@gmail.com>
Comment on lines +161 to +162
#' removed from the space. Options for the content are to: "leave"
#' leaving the content where it is, "archive" moving the content to
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing "leave" on its own made me realize it might be a bit counter-intuitive -- sounds like leaving a place more than leaving in place. I wonder if the option could be "keep" instead?

  • "keep": keep content where it is
  • "archive": move tp space archive
  • "trash": move to space trash

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those would make sense. I might re-map them here. It is a bit more difficult on my end to tweak them at this point.

Copy link
Collaborator

@mine-cetinkaya-rundel mine-cetinkaya-rundel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good so approving but I do recommend thinking about the "leave" option one more time, and seeing if "keep" might be more intuitive.

@ssinnott ssinnott merged commit 1c6c0a9 into master Oct 6, 2022
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.

Adjust space membership functions.
2 participants