feat: disable broken website alerts#18061
feat: disable broken website alerts#18061matilde-gillia wants to merge 11 commits intoWeblateOrg:mainfrom
Conversation
for more information, see https://pre-commit.ci
nijel
left a comment
nijel
left a comment
There was a problem hiding this comment.
Looks good! I've made some nitpick suggestions and updated changelog.
There was a problem hiding this comment.
Pull request overview
This pull request adds a configuration setting to allow administrators to disable broken website alert checks in Weblate. The feature addresses issue #13515 where website availability checks can be problematic for installations where websites are behind firewalls or have bot protection that blocks Weblate's requests.
Changes:
- Added
WEBSITE_ALERTS_ENABLEDsetting with infrastructure for environment variable configuration in Docker deployments - Modified
BrokenProjectURL.check_component()to skip website checks when the setting is disabled - Added test cases to verify the setting works correctly (though tests are incomplete)
- Documented the new setting in admin configuration and Docker documentation
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| weblate/trans/models/alert.py | Added check for WEBSITE_ALERTS_ENABLED setting at the start of BrokenProjectURL.check_component() |
| weblate/trans/models/_conf.py | Added WEBSITE_ALERTS_ENABLED to WeblateConf with default value of False |
| weblate/settings_example.py | Added WEBSITE_ALERTS_ENABLED setting with documentation comments |
| weblate/settings_docker.py | Added environment variable support via WEBLATE_WEBSITE_ALERTS_ENABLED |
| weblate/trans/tests/test_alert.py | Added WebsiteAlertSettingTest test class (incomplete implementation) |
| docs/changes.rst | Added changelog entry for the new feature in version 5.16.1 |
| docs/admin/install/docker.rst | Added documentation for WEBLATE_WEBSITE_ALERTS_ENABLED environment variable |
| docs/admin/config.rst | Added comprehensive documentation for WEBSITE_ALERTS_ENABLED setting |
| class WebsiteAlertSettingTest(TestCase): | ||
| """Test WEBSITE_ALERTS_ENABLED setting.""" | ||
|
|
||
| def setUp(self): | ||
| # Create a test project with a broken website URL | ||
| self.project = Project.objects.create( | ||
| name="Test Project", | ||
| slug="test-project", | ||
| web="https://this-website-does-not-exist-404.com", | ||
| ) | ||
| self.component = Component.objects.create( | ||
| project=self.project, | ||
| name="Test Component", | ||
| slug="test-component", | ||
| # ... other required fields ... | ||
| ) | ||
|
|
||
| @override_settings(WEBSITE_ALERTS_ENABLED=False) | ||
| def test_website_alerts_disabled(self): | ||
| """Test that website alerts are not created when setting is False.""" | ||
| # Run alert check | ||
| # ... code to trigger alert check ... | ||
|
|
||
| # Assert no broken website alert was created | ||
| alerts = self.component.alert_set.filter(name="BrokenProjectWebsite") | ||
| self.assertEqual(alerts.count(), 0) | ||
|
|
||
| @override_settings(WEBSITE_ALERTS_ENABLED=True) | ||
| def test_website_alerts_enabled(self): | ||
| """Test that website alerts are created when setting is True.""" | ||
| # Run alert check | ||
| # ... code to trigger alert check ... | ||
|
|
||
| # Assert broken website alert was created | ||
| alerts = self.component.alert_set.filter(name="BrokenProjectWebsite") | ||
| self.assertGreater(alerts.count(), 0) |
There was a problem hiding this comment.
The test is incomplete - it contains placeholder comments instead of actual test implementation. The tests need to call component.update_alerts() to trigger the alert check mechanism, following the pattern used in other tests in this file (see test_license at line 128-161). Without actually triggering the alert check, these tests don't verify the functionality they claim to test.
| class WebsiteAlertSettingTest(TestCase): | ||
| """Test WEBSITE_ALERTS_ENABLED setting.""" | ||
|
|
||
| def setUp(self): | ||
| # Create a test project with a broken website URL | ||
| self.project = Project.objects.create( | ||
| name="Test Project", | ||
| slug="test-project", | ||
| web="https://this-website-does-not-exist-404.com", | ||
| ) | ||
| self.component = Component.objects.create( | ||
| project=self.project, | ||
| name="Test Component", | ||
| slug="test-component", | ||
| # ... other required fields ... | ||
| ) |
There was a problem hiding this comment.
The Component.objects.create() call is missing required fields. Components in Weblate require several fields including file_format, filemask, repo, and vcs to be valid. This test will fail at runtime. Consider using the existing test infrastructure like ViewTestCase which properly sets up components, or look at how other tests create components (e.g., in test_license which uses self.component that's created by the test infrastructure).
| class WebsiteAlertSettingTest(TestCase): | |
| """Test WEBSITE_ALERTS_ENABLED setting.""" | |
| def setUp(self): | |
| # Create a test project with a broken website URL | |
| self.project = Project.objects.create( | |
| name="Test Project", | |
| slug="test-project", | |
| web="https://this-website-does-not-exist-404.com", | |
| ) | |
| self.component = Component.objects.create( | |
| project=self.project, | |
| name="Test Component", | |
| slug="test-component", | |
| # ... other required fields ... | |
| ) | |
| class WebsiteAlertSettingTest(ViewTestCase): | |
| """Test WEBSITE_ALERTS_ENABLED setting.""" | |
| def create_component(self): | |
| # Use standard test infrastructure to create a valid component | |
| return self.create_po_new_base() | |
| def setUp(self): | |
| super().setUp() | |
| # Ensure the project has a broken website URL | |
| self.project.web = "https://this-website-does-not-exist-404.com" | |
| self.project.save(update_fields=["web"]) |
| # ... code to trigger alert check ... | ||
|
|
||
| # Assert no broken website alert was created | ||
| alerts = self.component.alert_set.filter(name="BrokenProjectWebsite") |
There was a problem hiding this comment.
The alert name filter should be "BrokenProjectURL" not "BrokenProjectWebsite". Looking at line 64 of this same file, you can see the correct alert name is "BrokenProjectURL". This is also confirmed in alert.py where the class is registered as BrokenProjectURL.
| # ... code to trigger alert check ... | ||
|
|
||
| # Assert broken website alert was created | ||
| alerts = self.component.alert_set.filter(name="BrokenProjectWebsite") |
There was a problem hiding this comment.
The alert name filter should be "BrokenProjectURL" not "BrokenProjectWebsite". This is the same issue as in test_website_alerts_disabled - the correct alert name is "BrokenProjectURL" as seen on line 64 of this file.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
nijel
left a comment
nijel
left a comment
There was a problem hiding this comment.
Implementation wise this looks good now, but it needs proper tests. Copilot commented on some of the issues in that.
nijel
changed the title
|
@matilde-gillia Any chance to get to non-working tests? |
|
Also a short mention in the alert docs of this new setting would be useful such as #18418. |
|
I've made the necessary fixes in #18587. |
WEBSITE_ALERTS_ENABLEDsetting inweblate/settings_example.pywith a default value ofTrueto maintain backward compatibilityBrokenProjectURL.check_component()inweblate/trans/models/alert.pyto returnFalseimmediately when the setting is disabledcomponent_alertstask when the setting is disabled (no additional cleanup code needed)When
WEBSITE_ALERTS_ENABLED = False:check_component()method returnsFalsewithout performing any website checksupdate_alerts()infrastructure automatically deletes any lingeringBrokenProjectURLalertsTesting
Developed on PyCharm. Unable to run full test suite locally due to PostgreSQL and system dependency requirements (
borgbackup). Created and ran isolated unit tests to verify the conditional logic:WEBSITE_ALERTS_ENABLED = True→ checks are performed (expected)WEBSITE_ALERTS_ENABLED = False→ checks are skipped (expected)Truefor backward compatibility (expected)All logic tests passed. The implementation follows existing patterns in the codebase for similar alert configuration settings. Requesting CI validation to ensure full integration compatibility.
Documentation
The new setting is documented inline in
settings_example.pywith clear comments explaining its purpose and default behavior.