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 delete flag in configuration #707

Merged
merged 4 commits into from
Oct 30, 2017

Conversation

rpullinger
Copy link
Contributor

@rpullinger rpullinger commented Oct 17, 2017

- Summary
Fixes #593. Adds a delete flag to collections in config.yml. Defaults to false. This is mostly for use with files to restrict users from deleting settings files etc that are available via the CMS but can also be used to restrict the ability to delete within any collection.

I'm currently not 100% sure about the how it should handle delete not being set for a collection. Ideally I'd have it like I've implemented where if delete is not set then it counts as false. This is how the create flag works. My only worry is that when this feature is implemented and users upgrade they will suddenly find that unless they add the flag that they can't delete documents they could previously?

EDIT — This now defaults to true

- Test plan
Manually tested using example config using yarn run start.

- Description for the changelog
Adds a delete flag to collections in config.yml to restrict users from deleting important documents via the CMS.

Adds a `delete` flag to collections in `config.yml`. Defaults to
false. This is mostly for use with files to restrict users from
deleting settings files etc that available via the CMS.
@erquhart
Copy link
Contributor

@rpullinger awesome, looks good! One change, though: this needs to default to true, at least for now. We can consider changing it for the 1.0 release, but we want to avoid breaks unless absolutely necessary.

I would also say just add a single example of this option's usage in the example project, trying to reduce the signal-to-noise ratio in there.

Let me know if you have any other thoughts on any of this. Thanks again!

@Benaiah
Copy link
Contributor

Benaiah commented Oct 26, 2017

@rpullinger I've pushed the changes @erquhart requested into the delete-flag-in-configuration branch in this repo (netlify/netlify-cms) - if it works for you I can push them to this branch as well (or you can pull them from that branch) and we can merge (this LGTM once @erquhart's changes are addressed).

@rpullinger
Copy link
Contributor Author

rpullinger commented Oct 28, 2017

I've added @Benaiah's changes to the branch (cheers @Benaiah!) so that the default is now true. The config.yml now only has one example of the delete flag as well.

@erquhart – let me know if there is anything else?

Copy link
Contributor

@tech4him1 tech4him1 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@Benaiah Benaiah left a comment

Choose a reason for hiding this comment

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

LGTM

@Benaiah Benaiah merged commit a14f253 into decaporg:master Oct 30, 2017
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.

4 participants