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

Sync versions: use stable version instead of querying all versions #7380

Merged
merged 2 commits into from
Nov 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions readthedocs/api/v2/views/model_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ def sync_versions(self, request, **kwargs): # noqa: D205

# If the currently highest non-prerelease version is active, then make
# the new latest version active as well.
old_highest_version = determine_stable_version(project.versions.all())
if old_highest_version is not None:
activate_new_stable = old_highest_version.active
current_stable = project.get_original_stable_version()
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this checking for a new stable version (eg. the user has pushed a new higher version number), not getting it?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is done in the code below

# TODO: move this to an automation rule
promoted_version = project.update_stable_version()
new_stable = project.get_stable_version()
if promoted_version and new_stable and new_stable.active:
log.info(
'Triggering new stable build: %(project)s:%(version)s',
{
'project': project.slug,
'version': new_stable.identifier,
}
)
trigger_build(project=project, version=new_stable)
# Marking the tag that is considered the new stable version as
# active and building it if it was just added.
if (
activate_new_stable and
promoted_version.slug in added_versions
):

We just use this to get activate_new_stable

if current_stable is not None:
activate_new_stable = current_stable.active
else:
activate_new_stable = False

Expand Down
17 changes: 17 additions & 0 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,23 @@ def all_active_versions(self):
def get_stable_version(self):
return self.versions.filter(slug=STABLE).first()

def get_original_stable_version(self):
"""
Get the original version that stable points to.

Returns None if the current stable doesn't point to a valid version.
"""
current_stable = self.get_stable_version()
if not current_stable or not current_stable.machine:
return None
# Several tags can point to the same identifier.
# Return the stable one.
original_stable = determine_stable_version(
self.versions(manager=INTERNAL)
.filter(identifier=current_stable.identifier)
)
return original_stable

def update_stable_version(self):
"""
Returns the version that was promoted to be the new stable version.
Expand Down
15 changes: 12 additions & 3 deletions readthedocs/rtd_tests/tests/test_sync_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def setUp(self):
active=False,
type=TAG,
)
self.pip.update_stable_version()

def test_proper_url_no_slash(self):
version_post_data = {
Expand Down Expand Up @@ -86,6 +87,7 @@ def test_new_tag_update_active(self):
verbose_name='0.8.3',
active=True,
)
self.pip.update_stable_version()

version_post_data = {
'branches': [
Expand Down Expand Up @@ -132,6 +134,7 @@ def test_new_tag_dont_update_inactive(self):
type=TAG,
active=False,
)
self.pip.update_stable_version()

version_post_data = {
'branches': [
Expand Down Expand Up @@ -180,6 +183,7 @@ def test_delete_version(self):
verbose_name='0.8.3',
active=False,
)
self.pip.update_stable_version()

version_post_data = {
'branches': [
Expand Down Expand Up @@ -1088,13 +1092,14 @@ def test_update_stable_version(self):
],
}

self.pip.update_stable_version()
self.client.post(
'/api/v2/project/{}/sync_versions/'.format(self.pip.pk),
data=json.dumps(version_post_data),
content_type='application/json',
)

version_stable = Version.objects.get(slug=STABLE)
version_stable = self.pip.versions.get(slug=STABLE)
self.assertTrue(version_stable.active)
self.assertEqual(version_stable.identifier, '0.9')

Expand All @@ -1113,7 +1118,7 @@ def test_update_stable_version(self):
content_type='application/json',
)

version_stable = Version.objects.get(slug=STABLE)
version_stable = self.pip.versions.get(slug=STABLE)
self.assertTrue(version_stable.active)
self.assertEqual(version_stable.identifier, '1.0.0')

Expand All @@ -1132,7 +1137,7 @@ def test_update_stable_version(self):
content_type='application/json',
)

version_stable = Version.objects.get(slug=STABLE)
version_stable = self.pip.versions.get(slug=STABLE)
self.assertTrue(version_stable.active)
self.assertEqual(version_stable.identifier, '1.0.0')

Expand All @@ -1152,6 +1157,7 @@ def test_update_inactive_stable_version(self):
],
}

self.pip.update_stable_version()
self.client.post(
'/api/v2/project/{}/sync_versions/'.format(self.pip.pk),
data=json.dumps(version_post_data),
Expand Down Expand Up @@ -1192,6 +1198,7 @@ def test_stable_version_tags_over_branches(self):
{'identifier': '0.9', 'verbose_name': '0.9'},
],
}
self.pip.update_stable_version()

self.client.post(
'/api/v2/project/{}/sync_versions/'.format(self.pip.pk),
Expand Down Expand Up @@ -1233,6 +1240,7 @@ def test_stable_version_same_id_tag_branch(self):
],
}

self.pip.update_stable_version()
self.client.post(
'/api/v2/project/{}/sync_versions/'.format(self.pip.pk),
data=json.dumps(version_post_data),
Expand Down Expand Up @@ -1364,6 +1372,7 @@ def test_user_defined_stable_version_branch_with_tags(self):
active=True,
machine=True,
)
self.pip.update_stable_version()

version_post_data = {
'branches': [
Expand Down