feat(aci): Move monitor project/environment fields from the header to the form body#111762
feat(aci): Move monitor project/environment fields from the header to the form body#111762
Conversation
| const {project, detectorType} = useDetectorFormContext(); | ||
| const canEditDetector = useCanEditDetector({projectId: project.id, detectorType}); |
There was a problem hiding this comment.
Bug: The ProjectField validation checks permissions against the initial project from context, not the currently selected project in the form, leading to incorrect permission errors.
Severity: MEDIUM
Suggested Fix
The validation logic should get the currently selected project ID from the form state using useFormField('projectId') instead of relying on the stale project from DetectorFormContext. This will ensure permissions are checked against the user's actual selection.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/detectors/components/forms/common/projectField.tsx#L11-L12
Potential issue: The `ProjectField` component's `validate` function relies on
`canEditDetector`, which is calculated using the `project` from
`useDetectorFormContext`. This context is initialized once and is not updated when the
user changes the project in the dropdown. As a result, the permission check is always
performed against the initial project, not the one currently selected in the form. This
can either incorrectly block a valid submission or incorrectly allow a submission for a
project the user lacks permission to edit on the client side.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Hmm this is just code that I moved over so it's not related to theses changes, but there does appear to be a bug here.
Created a linear issue: ISWF-2330
saponifi3d
left a comment
There was a problem hiding this comment.
lgtm, i really like this moving towards single module exports (or as closely as we can with styled components).
Ref ISWF-2290
This is the first step of applying the refreshed monitor form designs - moving project/env out of the header and into the form body.
Previously, we were rendering these fields within the
DetectorBaseFieldscomponent which had title/proj/env, but this PR breaks that up into title, project, and env fields. Someone making a new form can choose between ProjectSection and ProjectEnvironmentSection depending on what is required.Before:

After:

And this is how the other monitor forms look now:


