feat(contact): creation - #15
Conversation
…tch Saudi Arabia, and subscriptions; enhance API integration and navigation for improved content administration
…ple components; add BulkActionsBar, BulkDeleteDialog, RowSelectCell, and SelectAllHeader for enhanced user experience in managing lists
PR Summary by QodoAdd four admin catalogs and reusable bulk actions
AI Description
Diagram
High-Level Assessment
Files changed (129)
|
Code Review by Qodo
1. Public contacts expose PII
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| public_contacts = public_uuid( | ||
| prefix="/public/contacts", | ||
| tags="public-contacts", | ||
| list_model=public.PublicContactList, |
There was a problem hiding this comment.
1. Public contacts expose pii 🐞 Bug ⛨ Security
The unauthenticated public contacts endpoints enumerate published records containing names, email addresses, phone numbers, companies, subjects, and message details. Publishing a contact therefore exposes private inquiry data to arbitrary callers.
Agent Prompt
## Issue description
The public contact list and detail routes expose inquiry PII without authentication. Remove these routes or require appropriate administrative authorization, and remove the corresponding public contract/schema if contacts are not genuinely public content.
## Issue Context
`public_uuid` has no authentication dependency, while `PublicContact` includes identifying and message data.
## Fix Focus Areas
- apps/Backend/src/flycatch_api/api/catalog.py[397-404]
- apps/Backend/src/flycatch_api/main.py[165-165]
- apps/Backend/src/flycatch_api/schemas/public_catalog.py[246-268]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1395-1410]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1466-1478]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| public_subscriptions = public_uuid( | ||
| prefix="/public/subscriptions", | ||
| tags="public-subscriptions", | ||
| list_model=public.PublicSubscriptionList, |
There was a problem hiding this comment.
2. Public subscriptions expose emails 🐞 Bug ⛨ Security
The unauthenticated public subscriptions endpoints enumerate subscriber email addresses, activity state, and creation timestamps. Publishing a subscription therefore discloses the mailing list to arbitrary callers.
Agent Prompt
## Issue description
The public subscription list and detail routes disclose subscriber email addresses without authentication. Remove these routes or require suitable administrative authorization, and remove the corresponding public contract/schema if subscription records are not public content.
## Issue Context
`public_uuid` has no authentication dependency, and `PublicSubscription` directly includes each subscriber's email address.
## Fix Focus Areas
- apps/Backend/src/flycatch_api/api/catalog.py[421-428]
- apps/Backend/src/flycatch_api/main.py[168-168]
- apps/Backend/src/flycatch_api/schemas/public_catalog.py[319-334]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1706-1723]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1765-1766]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def list_published(self, db: Session, q: str | None, page: int, per_page: int) -> PublicContactList: | ||
| query = db.query(ContactRow).filter(ContactRow.status == ContentStatus.publish) | ||
| page, per_page, rows, total = _paginate(query.order_by(ContactRow.created_at.desc()), page, per_page) | ||
| return PublicContactList(items=[self._public(row) for row in rows], page=page, per_page=per_page, total=total) |
There was a problem hiding this comment.
3. Public search parameter ignored 🐞 Bug ≡ Correctness
All four new list_published implementations accept q but never apply it to their queries. Consequently /public/contacts, /public/downloads, /public/flycatch-saudi-arabia, and /public/subscriptions return the same unfiltered results for every search term.
Agent Prompt
## Issue description
The four new public list services ignore the `q` value forwarded by the API. Apply entity-appropriate case-insensitive filters before pagination, consistent with existing catalog services, or remove `q` from the routes and contracts if search is unsupported.
## Issue Context
The shared public endpoint exposes `q` and passes it to each service, while existing catalog services filter their queries when it is non-empty.
## Fix Focus Areas
- apps/Backend/src/flycatch_api/services/catalog_service.py[1395-1398]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1497-1500]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1593-1600]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1706-1711]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| sa.UniqueConstraint("email"), | ||
| ) | ||
| op.create_index("ix_subscriptions_email", "subscriptions", ["email"]) |
There was a problem hiding this comment.
4. Email uniqueness allows case variants 🐞 Bug ☼ Reliability
SubscriptionService treats email uniqueness case-insensitively, but the database uses a normal case-sensitive PostgreSQL unique constraint. Concurrent or non-service writes can therefore store addresses such as User@example.com and user@example.com, violating the service's uniqueness rule.
Agent Prompt
## Issue description
The database constraint does not enforce the case-insensitive subscription email uniqueness implemented by the service. Add a unique expression index over `lower(email)` or use an equivalent case-insensitive database type, and align the ORM declaration.
## Issue Context
An application-level pre-check cannot protect concurrent requests or other database writers. Ensure integrity errors from duplicate concurrent writes are also converted to the documented validation response.
## Fix Focus Areas
- apps/Backend/alembic/versions/026_contacts_downloads_saudi_subscriptions.py[60-68]
- apps/Backend/src/flycatch_api/models/catalog.py[357-368]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1354-1363]
- apps/Backend/src/flycatch_api/services/catalog_service.py[1725-1738]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| _raise(error) | ||
| return Response(status_code=status.HTTP_204_NO_CONTENT) | ||
|
|
||
| attach_bulk_routes(router, resource=RESOURCE, model=BulkModel, not_found_key='admin.case_study_categories.not_found') |
There was a problem hiding this comment.
5. Referenced categories cause bulk failure 🐞 Bug ≡ Correctness
Bulk deletion of case-study categories, industries, and technologies bypasses their existing in-use checks and attempts direct deletion. Because their case-study link foreign keys do not cascade, deleting referenced records fails at commit with an unhandled integrity error instead of the established 409 response from the individual deletion endpoints.
Agent Prompt
## Issue description
Bulk deletion must enforce the same reference guards as single-record deletion for case-study categories, industries, and technologies so referenced records produce the established 409 response instead of an unhandled integrity error.
## Issue Context
`attach_bulk_routes` uses the generic direct ORM deletion path unless a `validate_delete` callback is supplied. The category, industry, and technology bulk routes supply no validator, bypassing the linked-case-study checks in their individual services; their non-cascading link foreign keys then cause deletion of referenced records to fail at commit.
## Fix Focus Areas
- apps/Backend/src/flycatch_api/api/admin_case_study_categories.py[95-95]
- apps/Backend/src/flycatch_api/services/case_study_category_service.py[117-135]
- apps/Backend/src/flycatch_api/api/admin_industries.py[86-86]
- apps/Backend/src/flycatch_api/services/industry_service.py[103-119]
- apps/Backend/src/flycatch_api/api/admin_technologies.py[89-89]
- apps/Backend/src/flycatch_api/services/technology_service.py[120-137]
- apps/Backend/src/flycatch_api/services/bulk_service.py[55-62]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2 participants
Changes
Reviewer
@bprahul017 & @Sinoj-flycatch