Skip to content

Remove PR Versions from Elasticsearch Indexing and HTMLFile Queryset Added #5805

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

Merged
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
2 changes: 1 addition & 1 deletion readthedocs/core/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def get_context_data(self, **kwargs):


def random_page(request, project_slug=None): # pylint: disable=unused-argument
html_file = HTMLFile.objects.order_by('?')
html_file = HTMLFile.objects.internal().order_by('?')
if project_slug:
html_file = html_file.filter(project__slug=project_slug)
html_file = html_file.first()
Expand Down
20 changes: 19 additions & 1 deletion readthedocs/projects/managers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
from django.db import models

from readthedocs.core.utils.extend import (
get_override_class,
SettingsOverrideObject
)
from readthedocs.projects.querysets import HTMLFileQuerySet

class HTMLFileManager(models.Manager):

class HTMLFileManagerBase(models.Manager):

@classmethod
def from_queryset(cls, queryset_class, class_name=None):
queryset_class = get_override_class(
HTMLFileQuerySet,
HTMLFileQuerySet._default_class, # pylint: disable=protected-access
)
return super().from_queryset(queryset_class, class_name)

def get_queryset(self):
return super().get_queryset().filter(name__endswith='.html')


class HTMLFileManager(SettingsOverrideObject):
_default_class = HTMLFileManagerBase
3 changes: 2 additions & 1 deletion readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
FeatureQuerySet,
ProjectQuerySet,
RelatedProjectQuerySet,
HTMLFileQuerySet,
)
from readthedocs.projects.templatetags.projects_tags import sort_version_aware
from readthedocs.projects.validators import (
Expand Down Expand Up @@ -1216,7 +1217,7 @@ class HTMLFile(ImportedFile):
class Meta:
proxy = True

objects = HTMLFileManager()
objects = HTMLFileManager.from_queryset(HTMLFileQuerySet)()

def get_processed_json(self):
"""
Expand Down
25 changes: 25 additions & 0 deletions readthedocs/projects/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.db.models import Q, OuterRef, Subquery, Prefetch
from guardian.shortcuts import get_objects_for_user

from readthedocs.builds.constants import EXTERNAL
from readthedocs.core.utils.extend import SettingsOverrideObject

from . import constants
Expand Down Expand Up @@ -213,3 +214,27 @@ def for_project(self, project):
Q(projects=project) |
Q(default_true=True, add_date__gt=project.pub_date),
).distinct()


class HTMLFileQuerySetBase(models.QuerySet):

def internal(self):
"""
HTMLFileQuerySet method that only includes internal version html files.

It will exclude pull request/merge request Version html files from the queries
and only include BRANCH, TAG, UNKONWN type Version html files.
"""
return self.exclude(version__type=EXTERNAL)

def external(self):
"""
HTMLFileQuerySet method that only includes external version html files.

It will only include pull request/merge request Version html files in the queries.
"""
return self.filter(version__type=EXTERNAL)


class HTMLFileQuerySet(SettingsOverrideObject):
_default_class = HTMLFileQuerySetBase
44 changes: 43 additions & 1 deletion readthedocs/rtd_tests/tests/test_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from readthedocs.builds.constants import EXTERNAL, BRANCH, TAG
from readthedocs.builds.models import Version, Build
from readthedocs.projects.constants import PUBLIC, PRIVATE, PROTECTED
from readthedocs.projects.models import Project
from readthedocs.projects.models import Project, HTMLFile


User = get_user_model()
Expand Down Expand Up @@ -211,3 +211,45 @@ def test_external_build_manager_with_public_with_user_and_project(self):
def test_external_build_manager_with_api(self):
self.assertNotIn(self.internal_builds, Build.external.api())
self.assertIn(self.external_version_build, Build.external.api())


class TestHTMLFileManager(TestCase):

fixtures = ['test_data']

def setUp(self):
self.user = User.objects.create(username='test_user', password='test')
self.client.login(username='test_user', password='test')
self.pip = Project.objects.get(slug='pip')
# Create a External Version. ie: pull/merge request Version.
self.external_version = get(
Version,
project=self.pip,
active=True,
type=EXTERNAL,
privacy_level=PUBLIC
)
self.html_file = HTMLFile.objects.create(
project=self.pip,
version=self.external_version,
name='file.html',
slug='file',
path='file.html',
md5='abcdef',
commit='1234567890abcdef',
)
self.internal_html_files = HTMLFile.objects.exclude(version__type=EXTERNAL)

def test_internal_html_file_queryset(self):
"""
It will exclude pull/merge request Version html files from the queries
and only include BRANCH, TAG, UNKONWN type Version files.
"""
self.assertNotIn(self.html_file, HTMLFile.objects.internal())

def test_external_html_file_queryset(self):
"""
It will only include pull/merge request Version html files in the queries.
"""
self.assertNotIn(self.internal_html_files, HTMLFile.objects.external())
self.assertIn(self.html_file, HTMLFile.objects.external())
2 changes: 1 addition & 1 deletion readthedocs/search/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def get_queryset(self):

# Do not index files that belong to non sphinx project
# Also do not index certain files
queryset = queryset.filter(
queryset = queryset.internal().filter(
project__documentation_type__contains='sphinx'
)

Expand Down
1 change: 0 additions & 1 deletion readthedocs/search/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from django.core.urlresolvers import reverse
from django_dynamic_fixture import G


from readthedocs.builds.models import Version
from readthedocs.projects.models import HTMLFile
from readthedocs.search.tests.utils import get_search_query_from_project_file
Expand Down
24 changes: 24 additions & 0 deletions readthedocs/search/tests/test_documents.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import pytest

from readthedocs.builds.constants import EXTERNAL
from readthedocs.projects.models import HTMLFile
from readthedocs.search.documents import PageDocument


@pytest.mark.django_db
@pytest.mark.search
class TestPageDocument:

def test_get_queryset_does_not_include_external_versions(self, project):
# turn version into PR Version
version = project.versions.all()[0]
version.type = EXTERNAL
version.save()

html_file = HTMLFile.objects.filter(
project__slug=project.slug, version=version
)
qs = PageDocument().get_queryset()

assert qs.model == HTMLFile
assert not set(html_file).issubset(set(qs))