-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add feature flipping models to Projects, plus example feature flip #3194
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
Changes from all commits
59cccbf
d251bbd
21bab41
954f632
e6d2bdc
adbe72a
dbdf59f
5aa4696
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# -*- coding: utf-8 -*- | ||
# Generated by Django 1.9.12 on 2017-10-27 00:17 | ||
from __future__ import unicode_literals | ||
|
||
from django.db import migrations | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('builds', '0003_add-cold-storage'), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name='APIVersion', | ||
fields=[ | ||
], | ||
options={ | ||
'proxy': True, | ||
}, | ||
bases=('builds.version',), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# -*- coding: utf-8 -*- | ||
# Generated by Django 1.9.12 on 2017-10-27 12:55 | ||
from __future__ import unicode_literals | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('projects', '0018_fix-translation-model'), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name='Feature', | ||
fields=[ | ||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), | ||
('feature_id', models.CharField(max_length=32, unique=True, verbose_name='Feature identifier')), | ||
('add_date', models.DateTimeField(auto_now_add=True, verbose_name='Date feature was added')), | ||
('default_true', models.BooleanField(default=False, verbose_name='Historical default is True')), | ||
], | ||
), | ||
migrations.AddField( | ||
model_name='feature', | ||
name='projects', | ||
field=models.ManyToManyField(blank=True, to='projects.Project'), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# -*- coding: utf-8 -*- | ||
# Generated by Django 1.9.12 on 2017-10-27 12:56 | ||
from __future__ import unicode_literals | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('projects', '0019_add-features'), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name='APIProject', | ||
fields=[ | ||
], | ||
options={ | ||
'proxy': True, | ||
}, | ||
bases=('projects.project',), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,10 +26,10 @@ | |
from readthedocs.projects.querysets import ( | ||
ProjectQuerySet, | ||
RelatedProjectQuerySet, | ||
ChildRelatedProjectQuerySet | ||
ChildRelatedProjectQuerySet, | ||
FeatureQuerySet, | ||
) | ||
from readthedocs.projects.templatetags.projects_tags import sort_version_aware | ||
from readthedocs.projects.utils import make_api_version | ||
from readthedocs.projects.version_handling import determine_stable_version, version_windows | ||
from readthedocs.restapi.client import api | ||
from readthedocs.vcs_support.backends import backend_cls | ||
|
@@ -639,9 +639,10 @@ def get_latest_build(self, finished=True): | |
return self.builds.filter(**kwargs).first() | ||
|
||
def api_versions(self): | ||
from readthedocs.builds.models import APIVersion | ||
ret = [] | ||
for version_data in api.project(self.pk).active_versions.get()['versions']: | ||
version = make_api_version(version_data) | ||
version = APIVersion(**version_data) | ||
ret.append(version) | ||
return sort_version_aware(ret) | ||
|
||
|
@@ -821,6 +822,57 @@ def add_comment(self, version_slug, page, content_hash, commit, user, text): | |
hash=content_hash, commit=commit) | ||
return node.comments.create(user=user, text=text) | ||
|
||
@property | ||
def features(self): | ||
return Feature.objects.for_project(self) | ||
|
||
def has_feature(self, feature_id): | ||
"""Does project have existing feature flag | ||
|
||
If the feature has a historical True value before the feature was added, | ||
we consider the project to have the flag. This is used for deprecating a | ||
feature or changing behavior for new projects | ||
""" | ||
return self.features.filter(feature_id=feature_id).exists() | ||
|
||
|
||
class APIProject(Project): | ||
|
||
"""Project proxy model for API data deserialization | ||
|
||
This replaces the pattern where API data was deserialized into a mocked | ||
:py:cls:`Project` object. This pattern was confusing, as it was not explicit | ||
as to what form of object you were working with -- API backed or database | ||
backed. | ||
|
||
This model preserves the Project model methods, allowing for overrides on | ||
model field differences. This model pattern will generally only be used on | ||
builder instances, where we are interacting solely with API data. | ||
""" | ||
|
||
features = [] | ||
|
||
class Meta: | ||
proxy = True | ||
|
||
def __init__(self, *args, **kwargs): | ||
self.features = kwargs.pop('features', []) | ||
# These fields only exist on the API return, not on the model, so we'll | ||
# remove them to avoid throwing exceptions due to unexpected fields | ||
for key in ['users', 'resource_uri', 'absolute_url', 'downloads', | ||
'main_language_project', 'related_projects']: | ||
try: | ||
del kwargs[key] | ||
except KeyError: | ||
pass | ||
super(APIProject, self).__init__(*args, **kwargs) | ||
|
||
def save(self, *args, **kwargs): | ||
return 0 | ||
|
||
def has_feature(self, feature_id): | ||
return feature_id in self.features | ||
|
||
|
||
@python_2_unicode_compatible | ||
class ImportedFile(models.Model): | ||
|
@@ -921,3 +973,63 @@ def delete(self, *args, **kwargs): # pylint: disable=arguments-differ | |
from readthedocs.projects import tasks | ||
broadcast(type='app', task=tasks.symlink_domain, args=[self.project.pk, self.pk, True]) | ||
super(Domain, self).delete(*args, **kwargs) | ||
|
||
|
||
@python_2_unicode_compatible | ||
class Feature(models.Model): | ||
|
||
"""Project feature flags | ||
|
||
Features should generally be added here as choices, however features may | ||
also be added dynamically from a signal in other packages. Features can be | ||
added by external packages with the use of signals:: | ||
|
||
@receiver(pre_init, sender=Feature) | ||
def add_features(sender, **kwargs): | ||
sender.FEATURES += (('blah', 'BLAH'),) | ||
|
||
The FeatureForm will grab the updated list on instantiation. | ||
""" | ||
|
||
# Feature constants - this is not a exhaustive list of features, features | ||
# may be added by other packages | ||
USE_SPHINX_LATEST = 'use_sphinx_latest' | ||
|
||
FEATURES = ( | ||
(USE_SPHINX_LATEST, _('Use latest version of Sphinx')), | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we store these on the model? We usually have a constants file, so it seems odd to break that convention. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the prescribed method of defining constants: |
||
|
||
projects = models.ManyToManyField( | ||
Project, | ||
blank=True, | ||
) | ||
# Feature is not implemented as a ChoiceField, as we don't want validation | ||
# at the database level on this field. Arbitrary values are allowed here. | ||
feature_id = models.CharField( | ||
_('Feature identifier'), | ||
max_length=32, | ||
unique=True, | ||
) | ||
add_date = models.DateTimeField( | ||
_('Date feature was added'), | ||
auto_now_add=True, | ||
) | ||
default_true = models.BooleanField( | ||
_('Historical default is True'), | ||
default=False, | ||
) | ||
|
||
objects = FeatureQuerySet.as_manager() | ||
|
||
def __str__(self): | ||
return "{0} feature".format( | ||
self.get_feature_display(), | ||
) | ||
|
||
def get_feature_display(self): | ||
"""Implement display name field for fake ChoiceField | ||
|
||
Because the field is not a ChoiceField here, we need to manually | ||
implement this behavior. | ||
""" | ||
return dict(self.FEATURES).get(self.feature_id, self.feature_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious that Django makes a migration for a proxy model. I assume it doesn't do anything to the DB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://stackoverflow.com/questions/37988914/why-does-django-create-migration-files-for-proxy-models seems to say it's so later migrations can reference it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i was surprised by this too at first, i didn't realize this when doing the integrations proxy models.