[OSDEV-OSDEV-1467] Fix blocking API facility create endpoint for 1.25#427
Conversation
React App | Jest test suite - Code coverage reportTotal: 27.8%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several updates for version 1.25.0 of the Open Supply Hub project, encompassing database migrations, new API endpoints, architectural changes, and bug fixes. Key changes include the addition of new API methods for moderation events and modifications to existing endpoints. The database schema is updated with new migrations, and the infrastructure is enhanced by upgrading RDS instance types and increasing memory allocation. Additionally, a new error handling mechanism is implemented to manage facility uploads during maintenance. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
src/django/api/views/facility/facilities_view_set.py (1)
580-584: LGTM: Clean implementation of maintenance mode toggle.The implementation correctly uses waffle's feature toggle to disable facility creation during maintenance. The error message is clear and user-friendly.
Consider these minor improvements:
- Extract the maintenance message to a constant or settings to make it configurable
- Add a logging statement to track when requests are blocked
+# At the top with other constants +MAINTENANCE_MODE_MESSAGE = ('Open Supply Hub is undergoing maintenance and ' + 'not accepting new data at the moment. Please ' + 'try again in a few minutes.') if switch_is_active('disable_list_uploading'): + log.info('[Maintenance Mode] Blocking facility creation request') - block_message = ('Open Supply Hub is undergoing maintenance and ' - 'not accepting new data at the moment. Please ' - 'try again in a few minutes.') - raise ServiceUnavailableException(block_message) + raise ServiceUnavailableException(MAINTENANCE_MODE_MESSAGE)doc/release/RELEASE-NOTES.md (4)
31-31: Consider adding the release status.For better tracking, consider adding a status indicator (e.g., "Released", "In Progress", "Planned") at the beginning of the release notes.
-* [OSDEV-1467](https://opensupplyhub.atlassian.net/browse/OSDEV-1467) - Implemented disabling endpoint `POST /api/facilities/?public=true&create=true` during the release process . It is raising an error message with status code 503. +Status: In Progress + +* [OSDEV-1467](https://opensupplyhub.atlassian.net/browse/OSDEV-1467) - Implemented disabling endpoint `POST /api/facilities/?public=true&create=true` during the release process. It is raising an error message with status code 503.
Line range hint
13-24: Enhance database migration documentation.The database migrations section could benefit from more detailed descriptions of the changes and their impact:
### Database changes #### Migrations: -* 0159_alter_status_of_moderation_events_table.py - This migration alters status of api_moderationevent table. -* 0160_allow_null_parsing_errors_in_facilitylist.py - This migration allows empty parsing_errors in api_facilitylist. -* 0161_create_disable_list_uploading_switch.py - This migration creates disable_list_uploading switch in the Django admin panel and record in the waffle_switch table. +* 0159_alter_status_of_moderation_events_table.py + - Modifies the status field in api_moderationevent table + - Impact: Affects how moderation event statuses are stored and processed + +* 0160_allow_null_parsing_errors_in_facilitylist.py + - Allows null values for parsing_errors in api_facilitylist + - Impact: Improves error handling flexibility during facility list processing + +* 0161_create_disable_list_uploading_switch.py + - Creates disable_list_uploading switch in Django admin panel + - Adds corresponding record in waffle_switch table + - Impact: Enables feature flag control for list upload functionality during releases
Line range hint
47-48: Enhance release instructions with verification steps.The release instructions should include steps to verify the successful application of changes.
### Release instructions: -* Ensure that the following commands are included in the `post_deployment` command: +### Release instructions: + +1. Pre-deployment checks: + * Verify database backup is available + * Ensure maintenance mode is enabled if needed + +2. Deployment steps: + * Ensure that the following commands are included in the `post_deployment` command: * `migrate` * `reindex_database` + +3. Post-deployment verification: + * Verify migrations completed successfully + * Check logs for any errors + * Test the new disable_list_uploading feature + * Verify moderation event functionality + * Confirm RDS instance upgrades completed + +4. Rollback plan: + * Document steps to revert migrations if needed + * Include commands to restore previous state
Line range hint
26-45: Improve organization of API and architecture changes.The code/API changes and architecture changes sections could be better organized for clarity.
### Code/API changes +#### New Endpoints * [OSDEV-1346](https://opensupplyhub.atlassian.net/browse/OSDEV-1346) - Create GET request for `v1/moderation-events` endpoint. * [OSDEV-1429](https://opensupplyhub.atlassian.net/browse/OSDEV-1429) - The list upload switcher has been created to disable the `Submit` button on the List Contribute page through the Switch page in the Django admin panel during the release process. Implemented a check on the list upload endpoint. + +#### Modified Endpoints * [OSDEV-1332](https://opensupplyhub.atlassian.net/browse/OSDEV-1332) - Introduced new `PATCH api/v1/moderation-events/{moderation_id}` endpoint to modify moderation event `status`. * [OSDEV-1347](https://opensupplyhub.atlassian.net/browse/OSDEV-1347) - Create GET request for `v1/moderation-events/{moderation_id}` endpoint. * Update `/v1/production-locations/{os_id}` endpoint to return a single object instead of multiple objects. Also, add unit tests for the `ProductionLocationsViewSet`. ### Architecture/Environment changes +#### Infrastructure Updates * The RDS instance has been upgraded as follows: for `production` and `preprod`, it is now `db.m6in.8xlarge`, and for `test`, it has been upgraded to `db.t3.xlarge`. + +#### Performance Improvements * Increased the memory for the Dedupe Hub instance from 8GB to 12GB in the `production` and `pre-prod` environments to reduce the risk of container overload and minimize the need for reindexing in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
doc/release/RELEASE-NOTES.md(1 hunks)src/django/api/views/facility/facilities_view_set.py(2 hunks)
🔇 Additional comments (2)
src/django/api/views/facility/facilities_view_set.py (2)
580-580: Verify feature toggle configuration.
Ensure the feature toggle 'disable_list_uploading' is properly configured in Django settings and consistently named across the codebase.
✅ Verification successful
Based on the results, I can generate the final response as we have comprehensive information about the feature toggle implementation.
Feature toggle 'disable_list_uploading' is properly configured and consistently used
The feature toggle is:
- Properly created through migration
0161_create_bdisable_list_uploading.py - Consistently named across both React and Django codebases
- Used in appropriate test cases with proper overrides
- Managed via Django's Waffle framework as evidenced by the migration and management command
- Has a default state of 'off' as set in the enable_switches management command
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check feature toggle configuration and usage
# Check for feature toggle configuration in settings files
echo "Checking feature toggle configuration..."
rg -l "disable_list_uploading" "src/django/*/settings*.py"
# Check for consistent feature toggle naming across codebase
echo "Checking feature toggle usage..."
rg "disable_list_uploading"
Length of output: 2153
14-14: Verify error handling implementation.
The ServiceUnavailableException import is correctly added. Let's verify its implementation and usage pattern.
✅ Verification successful
Error handling implementation is properly integrated
The verification shows:
- ServiceUnavailableException is correctly implemented as a Django APIException with HTTP 503 status code
- The exception is used consistently in facility-related views for maintenance scenarios:
- facility_list_view_set.py: Used during maintenance to block new data submissions
- facilities_view_set.py: Used with a configurable block message
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check ServiceUnavailableException implementation and usage
# Check ServiceUnavailableException implementation
echo "Checking ServiceUnavailableException implementation..."
rg -A 5 "class ServiceUnavailableException"
# Check for consistent error handling patterns
echo "Checking error handling patterns..."
ast-grep --pattern 'raise ServiceUnavailableException($$$)'
Length of output: 3183
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 56.14%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Django App | Unittest test suite - Code coverage reportTotal: 79.16%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
|
VadimKovalenkoSNF
left a comment
There was a problem hiding this comment.
Looks good, approved.



OSDEV-1467 Implemented disabling endpoint POST /api/facilities/ during the release process. It is raising an error message with status code 503.