Project: show search settings in separate form#12843
Conversation
|
@copilot write a test for this view |
…ew bugs (#12844) `ProjectSearchSettingsUpdate` had no tests and contained two bugs that were caught while writing them. ## Bugs fixed - **Missing `PrivateViewMixin`**: The view didn't inherit from `PrivateViewMixin`, so unauthenticated users were never redirected to login — they'd hit `get_object_or_404` directly and get a 404 instead of a redirect. - **`get_success_url()` crash on POST**: Used `self.object.slug`, but Django's `ModelFormMixin.form_valid()` reassigns `self.object = form.save()` — and since the form saves an `AddonsConfig` instance (not `Project`), this raised `AttributeError: 'AddonsConfig' object has no attribute 'slug'`. Fixed to use `self.get_project().slug`. ## Tests added `TestProjectSearchSettingsUpdate` in `readthedocs/projects/tests/test_views.py`: - `test_get_returns_200_for_admin` - `test_get_returns_404_for_non_admin` - `test_get_redirects_unauthenticated_user` - `test_post_updates_search_settings` — verifies `AddonsConfig.search_enabled` and `search_show_subprojects_filter` are persisted and redirects to the form URL - `test_post_updates_partial_search_settings` — verifies partial field updates work correctly ## Template Added a placeholder `projects/search_settings_form.html` following the same pattern as `pull_requests_form.html` (real implementation lives in ext-theme). <!-- START COPILOT CODING AGENT TIPS --> --- 📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs), [Azure Boards](https://gh.io/cca-azure-boards-docs) or [Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in one click without leaving your project management tool. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: stsewd <4975310+stsewd@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a dedicated “Search settings” page to the authenticated project admin area, separating search-modal configuration from the broader addons/project settings workflow.
Changes:
- Added a new private dashboard update view + URL route for per-project search settings.
- Introduced a focused
ModelFormto edit onlyAddonsConfigsearch-related fields. - Added view tests covering access control and POST updates; updated
search_enabledfield verbose name (and migration).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
readthedocs/projects/views/private.py |
Adds ProjectSearchSettingsUpdate view to update project search settings. |
readthedocs/projects/urls/private.py |
Registers the new /search-settings/ route in private project URLs. |
readthedocs/projects/tests/test_views.py |
Adds tests for GET/POST behavior and permissions for the new view. |
readthedocs/projects/forms.py |
Adds AddonsConfigSearchSettingsForm to edit only search-related AddonsConfig fields. |
readthedocs/projects/models.py |
Updates AddonsConfig.search_enabled verbose name (“Enable search modal”). |
readthedocs/projects/migrations/0159_update_addonsconfig_field_name.py |
Migration to apply the verbose name change to current + historical models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
agjohnson
left a comment
There was a problem hiding this comment.
This looks okayish for now, however I noted how we had originally planned on pages like a top-level search page for all search configuration options, not just Addons search configuration options.
So while we could start with this, we're also just going to end up winding back this pattern to make a generic form that supports changing fields on Project and Addons next.
If it's easy enough to figure this pattern out now, we could do that here.
Otherwise, I don't see us wanting to break out just the Addons subpages to top level for any other subpage. A good example is all the pull request options, these would be best on a similar form mixing the existing pull request form and Addons diff options.
I started this PR with support for multiple forms bfd6fce, but I reverted it back because we didn't really need the option from the project model yet. |
|
Yup, it might be a minute before we want this either way. When we do we are going to want one single form on the view, I don't think we use multiple forms on a single page anywhere. So this will be a bit of a new pattern and the forms might be a little more disconnected from the modeling. |
|
Would be good to merge this in, and update the docs to reference it. I'd also like to see a small header text on these pages, to make it a bit more usable for the user to understand what they can do here. |
We should be able to just have a single form that saves multiple objects? I don't think we need multiple forms. |
<img width="809" height="509" alt="image" src="https://github.com/user-attachments/assets/285e16c8-8613-47bd-ab20-34d91b01251e" /> ref readthedocs/readthedocs.org#12843
Co-authored-by: Anthony <aj@ohess.org>
|
Hmm, looks like an old installation of ext-theme is cached by circleci, and they don't have an interface to purge a cache :/ |
|
Tests are passing locally, merging. Will open another PR for the CI problem |
Follow-up to #12843, which moved project search configuration out of the Addons settings form into its own dedicated settings page. Update the server-side search, subprojects, and addons docs to point users to the new "Search" page under project Settings.
Follow-up to #12843, which moved project search configuration out of the Addons settings form into its own dedicated settings page. Update the server-side search, subprojects, and addons docs to point users to the new "Search" page under project Settings.
Follow-up to #12843, which moved project search configuration out of the Addons settings form into its own dedicated settings page. Update the server-side search, subprojects, and addons docs to point users to the new "Search" page under project Settings.
Follow-up to #12843, which moved project search configuration into its own dedicated settings page. Updates the existing "Configurable" bullet in the server-side search docs and the "Search" section in the subprojects docs to point readers at `Settings > Search`. Closes #11533. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>
I didn't include the option to disable search indexing here since its from a separate model and it's only shown when we have disabled search indexing, but isn't that complex to integrate if we wanted 11f16a1.
Closes #11533
Requires readthedocs/ext-theme#709