Add federated search and enhance API responses#2546
Conversation
…urce URL generation, and update resource type descriptions
44cbc67 to
d7c4158
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements federated search functionality to enable cross-site project discovery across multiple PhysioNet instances, while also enhancing the public API with additional metadata fields.
Key Changes:
- Add persistent UUID identifiers to published projects for stable cross-site references
- Implement federation models (FederatedSite, FederatedProject, FederationSyncLog) in the search app
- Enhance API responses to include resource_type, access_policy, and topics as human-readable strings
- Create management command for synchronizing project metadata from federated sites
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| physionet-django/project/modelcomponents/publishedproject.py | Adds public_project_uuid field to PublishedProject model for persistent identification |
| physionet-django/project/migrations/0087_publishedproject_public_project_uuid.py | Creates nullable UUID field on PublishedProject |
| physionet-django/project/migrations/0088_backfill_public_project_uuid.py | Backfills UUIDs for existing published projects |
| physionet-django/project/migrations/0089_alter_publishedproject_public_project_uuid.py | Enforces unique constraint and default value on UUID field |
| physionet-django/project/migrations/0090_merge_20260106_1200.py | Merge migration resolving parallel development branches |
| physionet-django/search/modelcomponents/federation.py | Defines federation models for sites, projects, and sync logs |
| physionet-django/search/modelcomponents/init.py | Package initialization for federation model components |
| physionet-django/search/models.py | Imports and exposes federation models |
| physionet-django/search/migrations/0001_initial.py | Creates database tables for federation models |
| physionet-django/search/management/commands/sync_federated_sites.py | Management command to synchronize metadata from federated sites |
| physionet-django/search/management/commands/init.py | Package initialization for management commands |
| physionet-django/search/management/init.py | Package initialization for management module |
| physionet-django/search/federation.py | Implements federated search query logic and scoring |
| physionet-django/search/admin.py | Registers federation models with Django admin interface |
| physionet-django/export/serializers.py | Adds ProjectFieldsMixin and exposes new fields in API responses |
| physionet-django/export/views.py | Updates API documentation and adds context to detail view |
| physionet-django/export/tests/test_views.py | Adds comprehensive tests for new API field serialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…uding filtering and display enhancements.
c2655ff to
d545350
Compare
d545350 to
5617ea0
Compare
|
@tompollard, @bemoody Please take a look - I think the reverse migrations is failing because of the retroactive uuid addition for published projects. Apart from that, things seems to be good to go. |
|
The |
tompollard
left a comment
There was a problem hiding this comment.
Very cool @Rutvikrj26. I successfully implemented federated search locally.
I added some comments inline.
| <small>Source: {{ published_project.source_site.site_name }}</small> | ||
| </p> | ||
| <div style="margin-bottom: 1rem;"> | ||
| {{ published_project.abstract|safe|truncatechars_html:250 }} |
There was a problem hiding this comment.
Should we remove "safe" here? I don't think we want unescaped HTML from external sites?
There was a problem hiding this comment.
I don't think it is a good idea.
- This might break the visual hierarchy since the local projects will still have the safe tag, but not the federated projects.
- The federated projects are trusted physionet instances. The idea is that those projects seamlessly blend in with the local projects, reducing the difference, making the search truly federated.
There was a problem hiding this comment.
Sanitize the incoming API response HTML content with bleach.
There was a problem hiding this comment.
The abstract is defined in FederatedProject as a TextField. One approach to sanitizing would be:
(a) change TextField to SafeHTMLField, and
(b) invoke full_clean before creating FederatedProject.
FederatedProject.objects.create does not invoke full_clean; it bypasses all model/field validation.
There was a problem hiding this comment.
Good Catch, Thanks!
Will open a quick pr for this.
bemoody
left a comment
There was a problem hiding this comment.
Haven't fully reviewed but here are some style things:
Put import statements at the top level; don't put them inside functions or classes. (console/forms.py, console/views.py)
Please don't do unrelated reformatting (project/modelcomponents/metadata.py, project/test_views.py, project/views.py, maybe other places?). Make those changes, if needed, in a separate pull request.
Don't use integer keys in URLs (console/urls.py). FederatedSite has a "site_identifier"; use that in the URL instead.
a6917ae to
890ccbc
Compare
|
Since pull #2574 was merged, there will be a migration conflict. You should be able to fix this by editing the dependencies in |
|
Hey @Rutvikrj26 please could you update the migrations when you get a chance? |
8dcaf33 to
11142a7
Compare
|
@tompollard I've recreated migrations. |
|
@Rutvikrj26 Can you check the migrations? Looks like the upgrade test is failing. |
|
@tompollard just fixed it. |
tompollard
left a comment
There was a problem hiding this comment.
Sorry, I notice a couple of issues.
|
@tompollard Thanks for catching these. |
4283e37 to
daa8c4d
Compare
|
My comment here (and Tom's) was not addressed. #2546 (comment) |
|
Good catch @bemoody, sorry for missing this. @Rutvikrj26 please could you raise an issue and add a fix in a new PR? |
Enhance API responses by including resource type, access policy, and topics. Implement federated search functionality with UUID support for project models.
API Updates
Federation Models Migration
Search Functionality
Management Commands
Admin Interface
Based on PR #2534 feedback from @bemoody: