feat: Added comprehensive test coverage for Group-related models #1020

zeus2611 · 2024-11-27T06:18:22Z

Contributor checklist

This pull request is on a separate branch and not the main branch
I have run the tests for the backend and frontend depending on what's needed for my changes as described in the testing section of the contributing guide

Description

This PR adds extensive test coverage for all Group-related models in the entities module. The new tests ensure proper validation, relationship handling, and data integrity across the Group model ecosystem.

Key additions:

GroupMember tests validating different role combinations (owner/admin/comms)
GroupEvent tests that multiple events can be associated with a single group
GroupResource tests covering creation and cascade deletion
GroupText tests for multi-language support and field constraints
GroupTopic tests ensuring proper group relationships
Group model tests for field validation and relationships

Test Coverage

New test files:

test_group_event.py
test_group_member.py
test_group_resource.py
test_group_text.py
test_group_topic.py
test_group.py

All tests use pytest and the Django test database marker.

Testing Notes

All tests can be run with standard pytest commands
Factory Boy factories are used for consistent test data
Database operations are properly marked with pytest.mark.django_db

Related issue

Implement groups api testing #828

Added following tests for Group model: - Group Creation: Tests successful creation, missing required fields, and invalid data. - Group Update: Tests successful update and handling of invalid input. - Group Deletion: Tests successful deletion and an attempt to delete a non-existent group.

- Add test_group_creation to validate complete Group object creation - Add test_required_fields to ensure validation for mandatory fields - Add test_optional_fields to verify handling of optional fields This commit introduces three unit tests for the Group model: 1. Validates full object creation with all fields 2. Checks validation errors for missing required fields 3. Confirms optional fields can be blank/null

Add tests to verify: - Maximum length constraints for Group model fields - Cascade deletion behavior when parent Organization or User is deleted

- Add test cases for GroupMember model covering roles and membership - Add test cases for GroupResource model with creation and deletion flows - Add test cases for GroupText model including language handling - Add test cases for GroupTopic model with relationship validations - Add test cases for GroupEvent model with multiple event relation - Expand Group model tests with field validations and cascade deletions The test suite now validates: - Model relationships and constraints - Field validations and max lengths - Multi-language support for text content - Cascade deletion behavior - Required vs optional field handling

github-actions · 2024-11-27T06:18:46Z

Thank you for the pull request!

The activist team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

The TypeScript and formatting workflows within the PR checks do not indicate new errors in the files changed
The Playwright end to end and Zap penetration tests have been ran and are passing (if necessary)
The changelog has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

netlify · 2024-11-27T06:19:28Z

✅ Deploy Preview for activist-org canceled.

Name	Link
🔨 Latest commit	`d960f41`
🔍 Latest deploy log	https://app.netlify.com/sites/activist-org/deploys/6747f8be49a48400087c1c48

andrewtavis · 2024-11-27T12:39:20Z

Thanks so much for this, @zeus2611! We'll take a look before the weekend :)

Would you have interest in looking into testing for other parts of the backend? If so, would be great if you'd make a suggestion, or @to-sta and I can also look into things. We'd make another issue regardless 😊

zeus2611 · 2024-11-27T14:15:48Z

Sure, @andrewtavis, I’d be happy to! During testing, I noticed the current coverage is around 80-83%. I can explore areas with lower coverage and work on improving them. I’m also open to any suggestions you or @to-sta might have—just let me know!

andrewtavis · 2024-11-27T19:40:26Z

.gitignore

@@ -41,3 +41,5 @@ dist

 # Node dependencies
 node_modules
+/.vscode


Checking in on these being added in @zeus2611 :) We do have settings.json and extensions.json version controlled, so should we maybe take these lines out?

I initially thought this folder was automatically modified and added to version control. However, since these files are already version-controlled, I’ll update the .gitignore file to exclude them. Thanks for pointing that out!

andrewtavis · 2024-11-27T19:41:15Z

.vscode/settings.json

@@ -1,5 +1,10 @@
 {
  "eslint.validate": ["javascript", "typescript", "vue"],
  "eslint.useFlatConfig": true,
-  "typescript.tsdk": "./frontend/node_modules/typescript/lib"
+  "typescript.tsdk": "./frontend/node_modules/typescript/lib",
+  "python.testing.pytestArgs": [


Could you give me a quick explanation of why these lines are needed, @zeus2611? Thank you!

VS Code automatically added these lines while I was working on the tests. It seems they were included as part of the editor's configuration updates. Let me know if you’d like me to investigate further!

Let's remove these then :) Can you do another commit to do this?

Ah you already did :)

andrewtavis · 2024-11-27T19:43:27Z

backend/entities/tests/test_group.py

+    org = OrganizationFactory(created_by=user)
+    fake = Faker()
+
+    # Test invalid URL


Minor thing that we do for comments is that if they're their own line as is the case here, then it'd be best if they were fully punctuated as this is Python standard :) If it's an inline comment, then no need for capitalization or a period at the end, but if it's on its own line then including both would be best :) I can also do this if you'd like me to!

Thanks, @andrewtavis! That makes sense. I'll update the comments to follow the Python standard, ensuring punctuation and capitalization where needed for standalone comments. Appreciate the heads-up! 😊

andrewtavis · 2024-11-27T19:44:09Z

Let me know on the above points and then I'll get to the full review, @zeus2611!

…ation and capitalization.

…ctivist into 828-group-model-tests

to-sta · 2024-12-11T21:55:36Z

backend/entities/tests/test_group.py

+    assert group.terms_checked is False
+
+
+def test_field_max_lengths() -> None:


Hi @zeus2611,

I reviewed your changes, and they generally look great! 😃 One suggestion before we merge: please remove the tests related to Django, such as those for max_length or auto_fields.

We don't need these tests because Django’s ORM and model field types are already well-tested by the framework itself. We rely on Django's built-in tests to ensure that things like max_length and auto_fields are functioning correctly. Additionally, during the code review process, we check for the correct usage of these features. If there were any issues, we would catch them at that stage.

By removing these tests, we keep the test suite focused on more application-specific logic, avoiding redundancy.

Thanks!

zeus2611 added 5 commits October 29, 2024 12:10

Merge branch 'activist-org:main' into 828-group-model-tests

d29d144

test: add Group model validation and cascade delete tests

493be86

Add tests to verify: - Maximum length constraints for Group model fields - Cascade deletion behavior when parent Organization or User is deleted

zeus2611 added 2 commits November 27, 2024 11:58

fix error: Function is missing a return type annotation [no-untyped-def]

cf46c3c

Merge branch 'activist-org:main' into 828-group-model-tests

88acfb9

andrewtavis requested review from to-sta and andrewtavis November 27, 2024 12:37

andrewtavis reviewed Nov 27, 2024

View reviewed changes

zeus2611 added 2 commits November 28, 2024 10:27

Modified comments in files to follow Python standard, ensuring punctu…

e9b2389

…ation and capitalization.

Merge branch '828-group-model-tests' of https://github.com/zeus2611/a…

d960f41

…ctivist into 828-group-model-tests

to-sta requested changes Dec 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added comprehensive test coverage for Group-related models #1020

feat: Added comprehensive test coverage for Group-related models #1020

zeus2611 commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited by andrewtavis

Loading

netlify bot commented Nov 27, 2024 •

edited

Loading

andrewtavis commented Nov 27, 2024

zeus2611 commented Nov 27, 2024

andrewtavis Nov 27, 2024

zeus2611 Nov 28, 2024

andrewtavis Nov 27, 2024

zeus2611 Nov 28, 2024

andrewtavis Nov 28, 2024

andrewtavis Nov 28, 2024

andrewtavis Nov 28, 2024

andrewtavis Nov 27, 2024

zeus2611 Nov 28, 2024

andrewtavis commented Nov 27, 2024

to-sta Dec 11, 2024

		assert group.terms_checked is False


		def test_field_max_lengths() -> None:

feat: Added comprehensive test coverage for Group-related models #1020

Are you sure you want to change the base?

feat: Added comprehensive test coverage for Group-related models #1020

Conversation

zeus2611 commented Nov 27, 2024 • edited Loading

Contributor checklist

Description

Test Coverage

Testing Notes

Related issue

github-actions bot commented Nov 27, 2024 • edited by andrewtavis Loading

Thank you for the pull request!

Maintainer checklist

netlify bot commented Nov 27, 2024 • edited Loading

✅ Deploy Preview for activist-org canceled.

andrewtavis commented Nov 27, 2024

zeus2611 commented Nov 27, 2024

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

andrewtavis commented Nov 27, 2024

Choose a reason for hiding this comment

zeus2611 commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited by andrewtavis

Loading

netlify bot commented Nov 27, 2024 •

edited

Loading