Fix a proxy model bug related to ad-free

[davidfischer]

This correctly initializes and tests the APIProject proxy model with respect to ad-free projects.

• Correctly set ad-free status based on data in the API response
• Adds a pretty minimal test for initializing the APIProject proxy models

Related to #4387

This has been edited...

[humitos]

I tried this today and it seems that there is still something weird with this code.

(Pdb) project_data
{'id': 220270, 'name': 'rtd-project-b', 'slug': 'rtd-project-b', 'description': '', 'language': 'en', 'programming_language': 'words', 'repo': 'https://github.com/humitos/rtd-project-b.git', 'repo_type': 'git', 'default_version': 'latest', 'default_branch': '', 'documentation_type': 'sphinx', 'users': [2], 'canonical_url': 'http://localhost:8000/docs/rtd-project-b/en/latest/', 'enable_epub_build': True, 'enable_pdf_build': True, 'conf_py_file': '', 'analytics_code': '', 'cdn_enabled': False, 'container_image': '', 'container_mem_limit': '', 'container_time_limit': '', 'install_project': False, 'use_system_packages': False, 'suffix': '.rst', 'skip': False, 'requirements_file': '', 'python_interpreter': 'python', 'features': [], 'has_valid_clone': True, 'has_valid_webhook': True, 'show_advertising': False}
(Pdb) APIProject(**project_data).show_advertising
True
(Pdb) APIProject(**project_data).ad_free
False
(Pdb)

In that example, we are instantiating the APIProject object with show_advertising=False but then it returns True when accessing to it.

I didn't find the reason of this yet, but we need to keep researching about it to make it work properly.

[davidfischer]

Thanks. I'll get a fix out for this shortly.

[humitos]

I just wrote a simple test case that shows this error:

diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py
index e4d5b371a..41a46fed0 100644
--- a/readthedocs/rtd_tests/tests/test_project.py
+++ b/readthedocs/rtd_tests/tests/test_project.py
@@ -16,7 +16,7 @@ from readthedocs.builds.constants import (
     BUILD_STATE_CLONING, BUILD_STATE_FINISHED, BUILD_STATE_TRIGGERED, LATEST)
 from readthedocs.builds.models import Build
 from readthedocs.projects.exceptions import ProjectConfigurationError
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import APIProject, Project
 from readthedocs.projects.tasks import finish_inactive_builds
 from readthedocs.rtd_tests.mocks.paths import fake_paths_by_regex
 
@@ -125,6 +125,46 @@ class TestProject(ProjectMixin, TestCase):
             self.pip.conf_file()
 
 
+class APIProxyObjectsTests(TestCase):
+
+    def test_api_project(self):
+        data = {
+            'id': 220270,
+            'name': 'project-name',
+            'slug': 'project-name',
+            'description': '',
+            'language': 'en',
+            'programming_language': 'words',
+            'repo': 'https://github.com/fake-repository.git',
+            'repo_type': 'git',
+            'default_version': 'latest',
+            'default_branch': '',
+            'documentation_type': 'sphinx',
+            'users': [2],
+            'canonical_url': 'http://localhost:8000/docs/project-name/en/latest/',
+            'enable_epub_build': True,
+            'enable_pdf_build': True,
+            'conf_py_file': '',
+            'analytics_code': '',
+            'cdn_enabled': False,
+            'container_image': '',
+            'container_mem_limit': '',
+            'container_time_limit': '',
+            'install_project': False,
+            'use_system_packages': False,
+            'suffix': '.rst',
+            'skip': False,
+            'requirements_file': '',
+            'python_interpreter': 'python',
+            'features': [],
+            'has_valid_clone': True,
+            'has_valid_webhook': True,
+            'show_advertising': False,
+        }
+        api_project = APIProject(**data)
+        self.assertFalse(api_project.show_advertising)
+        self.assertTrue(api_project.ad_free)
+
+
 class TestProjectTranslations(ProjectMixin, TestCase):
 
     def test_translations(self):

I didn't find any kind of test around APIProject or APIVersion so it may worth to have some for this.

[humitos]

Changes look good!

Just added some things to consider regarding styling and an extra check.

[humitos]

nit: I think it's better to use assertFalse and assertTrue for this cases.

[davidfischer]

Ya, you're right. I don't know why I did that.

[humitos]

Maybe adding an assert for the show_advertising here is good to check that our overriden method does work.

[davidfischer]

I updated the PR based on (great) feedback.

[humitos]

👍

[stsewd]

Any reason to uses assertEquals and not assertEqual?

[davidfischer]

Seems like assertEqual is preferred. I never gave it much thought but I think I use the plural with lists/sets/tuples. I'll switch to singular.