feat: add preview access for non-public forms in form-fields API - #17
Conversation
Reviewer's GuideExtends the form-fields API to support previewing non-public forms by authorized users, adds an Sequence diagram for form-fields API access and preview behaviorsequenceDiagram
actor User
participant ClientApp
participant FormFieldsView
participant FormEntryModel
participant AuthSystem
User->>ClientApp: Request form rendering for slug
ClientApp->>FormFieldsView: GET /api/fobi-form-fields/{slug}/
FormFieldsView->>FormEntryModel: get(slug=slug)
alt FormEntry exists
FormEntryModel-->>FormFieldsView: FormEntry
alt FormEntry is_public is true
FormFieldsView->>FormFieldsView: is_preview = false
FormFieldsView->>FormFieldsView: get_declared_fields(form_entry)
FormFieldsView->>FormFieldsView: _build_widget_map(form_entry)
FormFieldsView-->>ClientApp: 200 OK
ClientApp-->>User: Render form (is_preview=false)
else FormEntry is_public is false
alt User is_authenticated and has view_formentry permission
FormFieldsView->>AuthSystem: check has_perm view_formentry
AuthSystem-->>FormFieldsView: allowed
FormFieldsView->>FormFieldsView: is_preview = true
FormFieldsView->>FormFieldsView: get_declared_fields(form_entry)
FormFieldsView->>FormFieldsView: _build_widget_map(form_entry)
FormFieldsView-->>ClientApp: 200 OK (is_preview=true)
ClientApp-->>User: Render form with preview indicator
else User not authenticated or lacks permission
FormFieldsView-->>ClientApp: 404 NotFound
ClientApp-->>User: Show form not found
end
end
else FormEntry does not exist
FormEntryModel-->>FormFieldsView: DoesNotExist
FormFieldsView-->>ClientApp: 404 NotFound
ClientApp-->>User: Show form not found
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The implementation checks
request.user.has_perm("fobi.view_formentry"), but the task spec and acceptance criteria call forunfold_fobi.view_formentryproxy; aligning the permission codename and app label will avoid surprises and keep behavior consistent with the documented contract. - Switching from returning a JSON body
{"error": "Form not found"}to raisingNotFoundchanges the 404 response payload; if existing consumers rely on the previous error shape, consider preserving the response structure while still using DRF’sNotFoundstatus handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The implementation checks `request.user.has_perm("fobi.view_formentry")`, but the task spec and acceptance criteria call for `unfold_fobi.view_formentryproxy`; aligning the permission codename and app label will avoid surprises and keep behavior consistent with the documented contract.
- Switching from returning a JSON body `{"error": "Form not found"}` to raising `NotFound` changes the 404 response payload; if existing consumers rely on the previous error shape, consider preserving the response structure while still using DRF’s `NotFound` status handling.
## Individual Comments
### Comment 1
<location path="src/unfold_fobi/api/views.py" line_range="107-108" />
<code_context>
+ is_preview = False
+ if not form_entry.is_public:
+ if (
+ request.user.is_authenticated
+ and request.user.has_perm("fobi.view_formentry")
+ ):
+ is_preview = True
</code_context>
<issue_to_address>
**🚨 question (security):** Preview access is granted solely on a global permission, which may be too broad for private forms.
Using only the global `fobi.view_formentry` permission means any user with that permission can preview all non-public forms, regardless of ownership or relationship to the form. If previews are meant to be more restricted, consider changing this to an object-level or ownership-based check, or introducing a dedicated, narrower preview permission.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| request.user.is_authenticated | ||
| and request.user.has_perm("fobi.view_formentry") |
There was a problem hiding this comment.
🚨 question (security): Preview access is granted solely on a global permission, which may be too broad for private forms.
Using only the global fobi.view_formentry permission means any user with that permission can preview all non-public forms, regardless of ownership or relationship to the form. If previews are meant to be more restricted, consider changing this to an object-level or ownership-based check, or introducing a dedicated, narrower preview permission.
Summary by Sourcery
Allow preview access to non-public forms via the form-fields API while preserving existing behavior for public forms.
New Features:
is_previewflag in the form-fields API response to distinguish preview from public access.Documentation:
Tests:
is_previewbehavior.