Add development fixtures for #1350 & Add django-browser-reload #1346 #1401

Walexero · 2022-10-05T21:07:36Z

Description

Added development fixtures for issue #1350 by adding factory_boy to this project and defining it in the required apps.
Added django-browser-reload to this project with the provided installation instructions.

brylie

Please add a title and description that indicate what this pull request resolves.

Walexero · 2022-10-07T07:52:34Z

Title and Description has been added

brylie

The factories should reference the actual project models. They currently seem to be example codes from the Factory Boy documentation. Let's take the next step and create factories for the CiviWiki models found in the models.py files.

brylie · 2022-10-07T16:22:15Z

project/categories/factory.py

+# Another, different, factory for the same object
+class AdminFactory(factory.Factory):
+    class Meta:
+        model = models.User
+
+    first_name = 'Admin'
+    last_name = 'User'
+    admin = True


What is this factory used for?

i misunderstood, the required implementation for the factory, the new commit contains the right approach

brylie · 2022-10-07T16:22:58Z

project/categories/factory.py

+
+class UserFactory(factory.Factory):
+    class Meta:
+        model = models.User


What model in the categories app are we building this factory for? See project/categories/models.py where the models are defined.

brylie · 2022-10-07T16:23:47Z

project/notification/factory.py

+
+class UserFactory(factory.Factory):
+    class Meta:
+        model = models.User


What model in the notifications app are we building this factory for? See project/notifications/models.py where the models are defined.

brylie · 2022-10-07T16:24:05Z

project/notification/factory.py

+# Another, different, factory for the same object
+class AdminFactory(factory.Factory):
+    class Meta:
+        model = models.User
+
+    first_name = 'Admin'
+    last_name = 'User'
+    admin = True


This is probably unnecessary.

Suggested change

# Another, different, factory for the same object

class AdminFactory(factory.Factory):

class Meta:

model = models.User

first_name = 'Admin'

last_name = 'User'

admin = True

brylie · 2022-10-07T16:24:20Z

project/threads/factory.py

+
+class UserFactory(factory.Factory):
+    class Meta:
+        model = models.User


What model in the threads app are we building this factory for? See project/threads/models.py where the models are defined.

brylie · 2022-10-07T16:39:54Z

Converting to draft to indicate the PR is still in progress.

brylie · 2022-10-10T06:39:27Z

project/accounts/factory.py

-        model = models.User
+        model = models.Profile
+
+    user = factory.RelatedFactory(UserFactory, factory_related_name="profile")


Should this field be called "profile" since it has the related name "profile"?

Suggested change

user = factory.RelatedFactory(UserFactory, factory_related_name="profile")

profile = factory.RelatedFactory(UserFactory, factory_related_name="profile")

The field in the profile model is:
user = models.OneToOneField(User,
on_delete=models.CASCADE,
related_name="profile")

That was the reason i had it done in that way, i can't find enough resource to help with a OneToOneField using factory_boy plus i am conflicted as to if i have to also include a field in the UserFactory but the model.User only contains a link to the database

Also should i change the name of each field which has a related_name to the related_name

brylie · 2022-10-10T06:40:15Z

project/categories/factory.py

Add new line at end of file

Suggested change

name = factory.Iterator(["Politics","Voting","Referendum"])

name = factory.Iterator(["Politics","Voting","Referendum"])

brylie · 2022-10-10T06:40:31Z

project/notification/factory.py

+    civi = factory.SubFactory(CiviFactory)
+    activity_type = factory.fuzzy.FuzzyChoice(models.Notification.activity_CHOICES, getter=lambda c: c[0])
+    read = factory.Faker().pybool()


Add new line at end of file

Suggested change

read = factory.Faker().pybool()

read = factory.Faker().pybool()

brylie · 2022-10-10T06:40:48Z

project/shared_factory/shared_factory.py

+    def create(cls, _fake_time=None, **kwargs):
+        wrapper = partial(freeze_time,time_to_freeze=_fake_time) if _fake_time else suppress
+        with wrapper(_fake_time):
+            return super().create(**kwargs)


Make sure all files have an empty line at the end

Suggested change

return super().create(**kwargs)

return super().create(**kwargs)

Walexero · 2022-10-10T11:45:08Z

all the above issues have been addressed

gorkemarslan · 2022-10-10T11:45:29Z

project/accounts/factory.py

+        # Add the iterable of categores using bulk addition
+        self.categories.add(*extracted)
+
+    #taggablemanager()


Suggested change

#taggablemanager()

it is meant to refer to the tags field and to indicate its the code for that field

gorkemarslan · 2022-10-10T11:45:50Z

project/accounts/factory.py

+
+
+
+
+
+
+
+
+


Suggested change

gorkemarslan · 2022-10-10T11:46:20Z

project/notification/factory.py

+    activity_type = factory.fuzzy.FuzzyChoice(models.Notification.activity_CHOICES, getter=lambda c: c[0])
+    read = factory.Faker().pybool()
+
+


Suggested change

gorkemarslan · 2022-10-10T11:46:32Z

project/shared_factory/shared_factory.py

+
+

Suggested change

gorkemarslan · 2022-10-10T11:47:02Z

project/threads/factory.py

+        # Add the iterable of categores using bulk addition
+        self.facts.add(*extracted)
+
+    #taggablemanager()


Suggested change

#taggablemanager()

gorkemarslan · 2022-10-10T11:47:24Z

project/threads/factory.py

+    authors = factory.SubFactory(UserFactory)
+    thread = factory.SubFactory(ThreadFactory)
+
+    #taggablemanager() 


Suggested change

#taggablemanager()

would remove it

gorkemarslan · 2022-10-10T11:48:24Z

project/threads/factory.py

+
+

Suggested change

gorkemarslan · 2022-10-10T11:58:43Z

Could you update your branch to run the latest pipeline in our project? Otherwise, tests fail and we cannot see formating issues.

…iWiki#1350 and Add django-browser-reload CiviWiki#1346

Walexero · 2022-10-10T12:50:56Z

I synced my repo to include the latest changes, but it seems to have closed this pull request, is there anything i can do to solve this?

brylie · 2022-10-11T04:55:45Z

I reopened the pull request. It looks like the CI is failing with the following error.

ModuleNotFoundError: No module named 'django_browser_reload'

Walexero · 2022-10-11T09:39:00Z

okay, maybe i should try installing it with poetry if pip doesn't make the module findable

brylie · 2022-10-11T09:45:49Z

Yes, we use Poetry to manage the dependencies for this project.

…iWiki#1350 and Add django-browser-reload CiviWiki#1346

Walexero · 2022-10-11T11:58:28Z

i have added the changes with django-browser-reload added with poetry so i think that should solve the CI issue

Walexero · 2022-10-18T13:53:34Z

hello, do you still need me working on this issue?

pyproject.toml

Walexero · 2023-05-07T09:37:40Z

Hi, can we fix any issues and close this issue. if there's any steps you need me to take, or resolve any conflicts with the code i pushed. I am available to do that

brylie · 2023-05-07T19:17:37Z

It looks like there are many files with conflicts, unfortunately

Walexero · 2023-05-16T09:20:18Z

How should I go about resolving this or what should be done? If you can put me through

brylie marked this pull request as draft October 5, 2022 21:47

Walexero marked this pull request as ready for review October 6, 2022 09:58

brylie requested changes Oct 7, 2022

View reviewed changes

Walexero changed the title ~~Closes #1350 & #1346~~ Add development fixtures for #1350 & Add django-browser-reload #1346 Oct 7, 2022

brylie requested changes Oct 7, 2022

View reviewed changes

brylie marked this pull request as draft October 7, 2022 16:39

brylie mentioned this pull request Oct 7, 2022

Project stewardship - 2022, week 40 #1378

Closed

Walexero marked this pull request as ready for review October 10, 2022 00:46

brylie reviewed Oct 10, 2022

View reviewed changes

gorkemarslan requested changes Oct 10, 2022

View reviewed changes

Walexero closed this Oct 10, 2022

Walexero force-pushed the develop branch from cb75ef2 to 034d8df Compare October 10, 2022 12:38

Walexero added 2 commits October 10, 2022 08:40

Updated repo to upstream commits with fix for issue:CiviWiki#1346

462c77c

Synced repo with Upstream and Closes:Add development fixtures for Civ…

021531d

…iWiki#1350 and Add django-browser-reload CiviWiki#1346

brylie reopened this Oct 11, 2022

Synced repo with Upstream and Closes:Add development fixtures for Civ…

aa323d5

…iWiki#1350 and Add django-browser-reload CiviWiki#1346

Walexero requested a review from brylie October 18, 2022 13:53

Walexero requested review from gorkemarslan and brylie and removed request for brylie and gorkemarslan October 18, 2022 13:53

brylie requested changes Oct 18, 2022

View reviewed changes

pyproject.toml Show resolved Hide resolved

Added development fixtures for CiviWiki#1350

f20a8ef

	user = factory.RelatedFactory(UserFactory, factory_related_name="profile")
	profile = factory.RelatedFactory(UserFactory, factory_related_name="profile")

	name = factory.Iterator(["Politics","Voting","Referendum"])
	name = factory.Iterator(["Politics","Voting","Referendum"])

	read = factory.Faker().pybool()
	read = factory.Faker().pybool()

	return super().create(**kwargs)
	return super().create(**kwargs)

		activity_type = factory.fuzzy.FuzzyChoice(models.Notification.activity_CHOICES, getter=lambda c: c[0])
		read = factory.Faker().pybool()

Add development fixtures for #1350 & Add django-browser-reload #1346 #1401

Are you sure you want to change the base?

Add development fixtures for #1350 & Add django-browser-reload #1346 #1401

Conversation

Walexero commented Oct 5, 2022 • edited by brylie

Description

brylie left a comment

Choose a reason for hiding this comment

Walexero commented Oct 7, 2022

brylie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brylie commented Oct 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Walexero commented Oct 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gorkemarslan commented Oct 10, 2022

Walexero commented Oct 10, 2022

brylie commented Oct 11, 2022

Walexero commented Oct 11, 2022

brylie commented Oct 11, 2022

Walexero commented Oct 11, 2022

Walexero commented Oct 18, 2022

Walexero commented May 7, 2023

brylie commented May 7, 2023

Walexero commented May 16, 2023

Walexero commented Oct 5, 2022 •

edited by brylie