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

Exclude Spam projects count from total_projects count #5955

Merged
merged 7 commits into from Jul 22, 2019

Conversation

@Abhi-khandelwal
Copy link
Contributor

@Abhi-khandelwal Abhi-khandelwal commented Jul 18, 2019

Fixes #5927

@@ -42,7 +42,7 @@ def get_context_data(self, **kwargs):
"""Add latest builds and featured projects."""
context = super().get_context_data(**kwargs)
context['featured_list'] = Project.objects.filter(featured=True)
context['projects_count'] = Project.objects.count()
context['projects_count'] = Project.objects.exclude(slug="spam").count()
Copy link
Member

@dojutsu-user dojutsu-user Jul 18, 2019

I think we want to have something like this:

queryset = queryset.exclude(users__profile__banned=True)

@saadmk11
Copy link
Member

@saadmk11 saadmk11 commented Jul 18, 2019

You may want to add tests for these changes. :)

@Abhi-khandelwal
Copy link
Contributor Author

@Abhi-khandelwal Abhi-khandelwal commented Jul 18, 2019

@saadmk11 Can you help me with these test changes, I mean where should I have to make changes.

@saadmk11
Copy link
Member

@saadmk11 saadmk11 commented Jul 18, 2019

you can add something similar to this for homepage with projects in test_views.py

def test_project_downloads_only_shows_internal_versons(self):
url = reverse('project_downloads', args=[self.pip.slug])
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertNotIn(self.external_version, response.context['versions'])

With self.assertEqual(<expected_number_of_projects>, response.context['projects_count'])

@Abhi-khandelwal
Copy link
Contributor Author

@Abhi-khandelwal Abhi-khandelwal commented Jul 18, 2019

@saadmk11 I got this, but still I have one confusion, Do I have to make seaparate class for this or I have to make a function test_projects_count() under RandomPageTests class?

@saadmk11
Copy link
Member

@saadmk11 saadmk11 commented Jul 18, 2019

you can make something like HomepageViewTests

@Abhi-khandelwal
Copy link
Contributor Author

@Abhi-khandelwal Abhi-khandelwal commented Jul 19, 2019

@dojutsu-user , I would like you to review on above changes :)

Copy link
Member

@dojutsu-user dojutsu-user left a comment

This is coming closer. Few comments.

@@ -277,8 +277,8 @@ def test_build_redirect(self, mock):
r._headers['location'][1],
'/projects/pip/builds/%s/' % build.pk,
)

def test_build_list_includes_external_versions(self):

Copy link
Member

@dojutsu-user dojutsu-user Jul 19, 2019

Remove this space.


def test_build_list_includes_external_versions(self):
def test_build_list_includes_external_versions(self):
Copy link
Member

@dojutsu-user dojutsu-user Jul 19, 2019

here also.

@@ -294,5 +294,15 @@ def test_build_list_includes_external_versions(self):
reverse('builds_project_list', args=[self.pip.slug]),
)
self.assertEqual(response.status_code, 200)

Copy link
Member

@dojutsu-user dojutsu-user Jul 19, 2019

here too

readthedocs/rtd_tests/tests/test_views.py Show resolved Hide resolved
Copy link
Member

@humitos humitos left a comment

Changes look good to me!

@@ -277,7 +278,7 @@ def test_build_redirect(self, mock):
r._headers['location'][1],
'/projects/pip/builds/%s/' % build.pk,
)

Copy link
Member

@stsewd stsewd Jul 22, 2019

Suggested change

stsewd
stsewd approved these changes Jul 22, 2019
Copy link
Member

@dojutsu-user dojutsu-user left a comment

LGTM..!!

Copy link
Member

@saadmk11 saadmk11 left a comment

Looks Good. Thanks for the update 👍

@stsewd stsewd merged commit e2a887b into readthedocs:master Jul 22, 2019
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants