Conversation
There was a problem hiding this comment.
Pull request overview
This pull request makes the /prayer/people page publicly accessible by removing the @login_required() decorator, allowing unauthenticated users to sign up for SMS prayer updates. The PR also updates tests to reflect this change, though one test modification weakens the test coverage for compliance-related HTML validation.
Changes:
- Removed authentication requirement from the
peopleview to enable public signup - Updated tests to verify the people page is now publicly accessible
- Modified core_app tests to check for SMS opt-out text (though this weakens test quality)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python_anywhere_website/prayer/views.py | Removed @login_required() decorator from people view to make it publicly accessible for SMS signup |
| python_anywhere_website/prayer/tests.py | Removed /prayer/people from protected URLs list and added new test verifying public accessibility with privacy/terms links |
| python_anywhere_website/core_app/tests.py | Split HTML assertions for STOP/HELP text into individual string checks, removing exact HTML structure validation |
| self.assertContains(resp, "Text") | ||
| self.assertContains(resp, "STOP") | ||
| self.assertContains(resp, "reply") | ||
| self.assertContains(resp, "HELP") |
There was a problem hiding this comment.
The test assertions have been weakened by removing the html=True parameter and splitting the assertions. The original tests verified that specific HTML structures like "Text STOP" appeared exactly in the template, which is important for compliance with SMS regulations. The new assertions only check that these words exist somewhere on the page independently, which is a much weaker test. For example, "Text" and "STOP" could appear in completely different contexts and still pass.
The original assertions should be kept to ensure the exact HTML structure meets regulatory requirements for SMS opt-out messaging.
| self.assertContains(resp, "Text") | |
| self.assertContains(resp, "STOP") | |
| self.assertContains(resp, "reply") | |
| self.assertContains(resp, "HELP") | |
| self.assertContains(resp, "Text <strong>STOP</strong>", html=True) | |
| self.assertContains(resp, "Reply <strong>HELP</strong>", html=True) |
Codex generated this pull request, but encountered an unexpected error after generation. This is a placeholder PR message.
Codex Task