From 1a1326172566c6d271a5141f7b5fc678509aaeeb Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 20 Jun 2019 14:58:37 +0200 Subject: [PATCH 1/6] Hide "Protected" privacy level from users 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 https://github.com/rtfd/readthedocs.org/issues/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. --- docs/privacy.rst | 16 +--------------- readthedocs/builds/forms.py | 3 ++- readthedocs/core/mixins.py | 21 +++++++++++++++++++++ readthedocs/projects/forms.py | 3 ++- readthedocs/projects/models.py | 3 +-- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/docs/privacy.rst b/docs/privacy.rst index 71b486d3729..bdcf3983545 100644 --- a/docs/privacy.rst +++ b/docs/privacy.rst @@ -2,7 +2,7 @@ Privacy Levels ============== Read the Docs supports 3 different privacy levels on 2 different objects; -Public, Protected, Private on Projects and Versions. +Public, Private on Projects and Versions. Understanding the Privacy Levels -------------------------------- @@ -12,8 +12,6 @@ Understanding the Privacy Levels +============+============+===========+===========+=============+ | Private | No | No | No | Yes | +------------+------------+-----------+-----------+-------------+ -| Protected | Yes | No | No | Yes | -+------------+------------+-----------+-----------+-------------+ | Public | Yes | Yes | Yes | Yes | +------------+------------+-----------+-----------+-------------+ @@ -27,18 +25,6 @@ Public This is the easiest and most obvious. It is also the default. It means that everything is available to be seen by everyone. -Protected -~~~~~~~~~ - -Protected means that your object won't show up in Listing Pages, -but Detail pages still work. For example, a Project that is Protected will -not show on the homepage Recently Updated list, however, -if you link directly to the project, you will get a 200 and the page will display. - -Protected Versions are similar, they won't show up in your version listings, -but will be available once linked to. - - Private ~~~~~~~ diff --git a/readthedocs/builds/forms.py b/readthedocs/builds/forms.py index 406da7417ab..08b0f7dd8ad 100644 --- a/readthedocs/builds/forms.py +++ b/readthedocs/builds/forms.py @@ -6,10 +6,11 @@ from django.utils.translation import ugettext_lazy as _ from readthedocs.builds.models import Version +from readthedocs.core.mixins import HideProtectedLevelMixin from readthedocs.core.utils import trigger_build -class VersionForm(forms.ModelForm): +class VersionForm(HideProtectedLevelMixin, forms.ModelForm): class Meta: model = Version diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index 4d6b1160c1c..17a4fc9f2e1 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -4,8 +4,11 @@ from django.contrib.auth.decorators import login_required from django.utils.decorators import method_decorator +from django.utils.translation import ugettext_lazy as _ from vanilla import ListView +from readthedocs.projects.constants import PRIVACY_CHOICES, PROTECTED + class ListViewWithForm(ListView): @@ -22,3 +25,21 @@ class LoginRequiredMixin: @method_decorator(login_required) def dispatch(self, *args, **kwargs): return super().dispatch(*args, **kwargs) + + +class HideProtectedLevelMixin: + + """ + Hide ``protected`` privacy level from Form. + + 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 + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + privacy_level = list(PRIVACY_CHOICES) + privacy_level.remove((PROTECTED, _('Protected'))) + self.fields['privacy_level'].choices = privacy_level diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 416b2e7c5df..056862ede8f 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -14,6 +14,7 @@ from guardian.shortcuts import assign from textclassifier.validators import ClassifierValidator +from readthedocs.core.mixins import HideProtectedLevelMixin from readthedocs.core.utils import slugify, trigger_build from readthedocs.core.utils.extend import SettingsOverrideObject from readthedocs.integrations.models import Integration @@ -189,7 +190,7 @@ def clean_tags(self): return tags -class ProjectAdvancedForm(ProjectTriggerBuildMixin, ProjectForm): +class ProjectAdvancedForm(HideProtectedLevelMixin, ProjectTriggerBuildMixin, ProjectForm): """Advanced project option form.""" diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index e4c90e1e72d..e87bc1716b7 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -329,8 +329,7 @@ class Project(models.Model): choices=constants.PRIVACY_CHOICES, default=settings.DEFAULT_PRIVACY_LEVEL, help_text=_( - 'Level of privacy that you want on the repository. ' - 'Protected means public but not in listings.', + 'Level of privacy that you want on the repository.', ), ) version_privacy_level = models.CharField( From 992e7ffba3ea96a17a3610568c11afa6017b1b30 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 1 Jul 2019 10:12:31 +0200 Subject: [PATCH 2/6] Keep showing Protected only for objects that are already set as that --- readthedocs/core/mixins.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index 17a4fc9f2e1..68149d5195a 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -33,13 +33,21 @@ class HideProtectedLevelMixin: Hide ``protected`` privacy level from Form. 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 + + If the current ``privacy_level`` is ``protected`` we show it (so users keep + seeing consistency values), and hide it otherwise (so it can't be selected). + + There is a better way to manage this by using Version states See: + https://github.com/rtfd/readthedocs.org/issues/5321 """ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - privacy_level = list(PRIVACY_CHOICES) - privacy_level.remove((PROTECTED, _('Protected'))) - self.fields['privacy_level'].choices = privacy_level + if any([ + not self.instance, + self.instance and self.instance.privacy_level != PROTECTED, + ]): + privacy_level = list(PRIVACY_CHOICES) + privacy_level.remove((PROTECTED, _('Protected'))) + self.fields['privacy_level'].choices = privacy_level From b0d587e69f699b18fa0e8d87f6333a0fc1f0cb4a Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 2 Jul 2019 16:36:01 +0200 Subject: [PATCH 3/6] Add test case for privacy_level Form choices --- .../rtd_tests/tests/test_project_forms.py | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index 50d9815d051..61835253dc8 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -2,13 +2,14 @@ from django.contrib.auth.models import User from django.test import TestCase from django.test.utils import override_settings +from django.utils.translation import ugettext_lazy as _ from django_dynamic_fixture import get from textclassifier.validators import ClassifierValidator - from readthedocs.builds.constants import LATEST, STABLE from readthedocs.builds.models import Version from readthedocs.projects.constants import ( PRIVATE, + PRIVACY_CHOICES, PROTECTED, PUBLIC, REPO_TYPE_GIT, @@ -272,6 +273,28 @@ def test_default_version_field_if_no_active_version(self): self.assertTrue(form.fields['default_version'].widget.attrs['readonly']) self.assertEqual(form.fields['default_version'].initial, 'latest') + def test_hide_protected_privacy_level_new_objects(self): + """ + Test PROTECTED is only allowed in old objects. + + New projects are not allowed to set the privacy level as protected. + """ + # New default object + project = get(Project) + form = ProjectAdvancedForm(instance=project) + + privacy_choices = list(PRIVACY_CHOICES) + privacy_choices.remove((PROTECTED, _('Protected'))) + self.assertEqual(form.fields['privacy_level'].choices, privacy_choices) + + # "Old" object with privacy_level previously set as protected + project = get( + Project, + privacy_level=PROTECTED, + ) + form = ProjectAdvancedForm(instance=project) + self.assertEqual(form.fields['privacy_level'].choices, list(PRIVACY_CHOICES)) + class TestProjectAdvancedFormDefaultBranch(TestCase): From ca39628c9aa7a04b11cb3f52771a36c708f92efe Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 2 Jul 2019 16:37:21 +0200 Subject: [PATCH 4/6] Docstring typos --- readthedocs/core/mixins.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index 68149d5195a..6332b7d4ac7 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -32,13 +32,13 @@ class HideProtectedLevelMixin: """ Hide ``protected`` privacy level from Form. - Remove Protected for now since it cause confusions to users. + Remove Protected for now since it causes confusions to users. If the current ``privacy_level`` is ``protected`` we show it (so users keep seeing consistency values), and hide it otherwise (so it can't be selected). - There is a better way to manage this by using Version states See: - https://github.com/rtfd/readthedocs.org/issues/5321 + There is a better way to manage this by using Version states. + See: https://github.com/rtfd/readthedocs.org/issues/5321 """ def __init__(self, *args, **kwargs): From a548b922897c95d46999c950898ca546cd61b4b5 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 2 Jul 2019 16:38:30 +0200 Subject: [PATCH 5/6] Simplify logic --- readthedocs/core/mixins.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index 6332b7d4ac7..a2e2af09328 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -44,10 +44,7 @@ class HideProtectedLevelMixin: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - if any([ - not self.instance, - self.instance and self.instance.privacy_level != PROTECTED, - ]): + if self.instance is None or self.instance.privacy_level != PROTECTED: privacy_level = list(PRIVACY_CHOICES) privacy_level.remove((PROTECTED, _('Protected'))) self.fields['privacy_level'].choices = privacy_level From 0959af7ee0c45a4fad213a2bec3ddd6e10fc4552 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 3 Jul 2019 15:01:49 +0200 Subject: [PATCH 6/6] Adding a dumb migration Only the help_text field changed (removed the protected sentence). --- .../migrations/0044_auto_20190703_1300.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 readthedocs/projects/migrations/0044_auto_20190703_1300.py diff --git a/readthedocs/projects/migrations/0044_auto_20190703_1300.py b/readthedocs/projects/migrations/0044_auto_20190703_1300.py new file mode 100644 index 00000000000..ae08a531eb6 --- /dev/null +++ b/readthedocs/projects/migrations/0044_auto_20190703_1300.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.21 on 2019-07-03 13:00 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0043_add-build-field'), + ] + + operations = [ + migrations.AlterField( + model_name='project', + name='privacy_level', + field=models.CharField(choices=[('public', 'Public'), ('protected', 'Protected'), ('private', 'Private')], default='public', help_text='Level of privacy that you want on the repository.', max_length=20, verbose_name='Privacy Level'), + ), + ]