Hide "Protected" privacy level from users #5833
Conversation
readthedocs/projects/constants.py
Outdated
| # Remove Protected for now since it cause confusions to users. | ||
| # There is a better way to manage this by using Version states | ||
| # See: https://github.com/rtfd/readthedocs.org/issues/5321 | ||
| # (PROTECTED, _('Protected')), |
There was a problem hiding this comment.
What do people with Protected in the DB currently see for this listing?
There was a problem hiding this comment.
Weird stuffs :)
Some places it will say protected (version listing) and some other Public (version details).
It may be better to remove it from the Form itself by overriding the field instead of from this global constant.
There was a problem hiding this comment.
Yeah, i'd have the same reservations around just removing it from the database full stop. I think we might need to be more gentle here, or perhaps even use a feature flag to grandfather projects into protected.
We also discussed maybe researching how projects are using the feature now, and probably messaging the users before making a hard change like this one.
There was a problem hiding this comment.
I'm not too worry about asking users how they use protected because it's definitely a weird concept and it's currently broken.
I don't want to remove it from the DB either, I just want to hide it from the users so they can not use Protected anymore in new projects/versions and get confused with it. The old ones should remain as they are: "in a Protected privacy level, but broken"
Protected causes a lot of confusions to users. I'm hiding it from the Form for now as a temporarily solution while we implement more Version states and this behavior can be easily managed. See #5321 Project/Version that are Protected will remain in the same state and everything should keep working. Although, new Project/Version are not allowed to set as Protected.
humitos
force-pushed
the
humitos/hide-protected-privacy-level
branch
from
3d9992c to
1a13261
Compare
|
I update the PR. This is better than what we have. Version/Project listing will say |
humitos
removed
the
PR: work in progress
|
If this "partial solution" does not fulfill what we want, we should just close the PR and spend time on the real solution. |
readthedocs/core/mixins.py
Outdated
| if any([ | ||
| not self.instance, | ||
| self.instance and self.instance.privacy_level != PROTECTED, | ||
| ]): |
There was a problem hiding this comment.
This logic feels quite complex. Can we not just do something like:
if self.instance is None or self.instance.privacy_level != PROTECTED
|
I added a test case for this and simplify the if logic as you mentioned. We can merge after the tests pass. |
Only the help_text field changed (removed the protected sentence).
Protected privacy level causes a lot of confusions to users. I'm hiding it for now as a temporarily solution while we implement more Version states and this behavior can be easily managed. See #5321
All the logic around Protect versions keeps working as is. Although, Projects and Versions are not being able to be added as
Protectedonce this PR gets merged.