Add incident creation API endpoint #1056
Conversation
✅ Deploy Preview for atlos-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -209,43 +238,50 @@ defmodule Platform.Material.Media do | |||
| changeset | |||
| end | |||
|
|
|||
| def validate_project(changeset, user \\ nil, media \\ nil) do | |||
| def validate_project(changeset, media \\ [], opts) do | |||
There was a problem hiding this comment.
this function is confusing to me — so much is going on. what is the purpose of this function? to make sure that the user or token can edit media within a project?
There was a problem hiding this comment.
this function would benefit a lot from either a comment or a rename
There was a problem hiding this comment.
considering it doesn't operate on projects but media, the name seems confusing at best and wrong at worst
There was a problem hiding this comment.
(to be clear, I wrote this function)
There was a problem hiding this comment.
Agree the name is cursed.💀 Just added a short comment to reflect that validate_project validates changes to an incident's project, not the project itself.
There was a problem hiding this comment.
(Working on a deeper change to this function so that we actually do validate that the API Token is permitted to edit the incident's project.)
Co-authored-by: R. Miles McCain <[email protected]>
Co-authored-by: R. Miles McCain <[email protected]>
Pattern matching failed under the simplified version.
Note: This is my first change to Atlos that involves write access and permissions changes. While I've approached this feature thoughtfully, it involves changes to fairly high-risk code paths and so is deserving of an extra layer of scrutiny before merging.
Changes
This PR involves four main types of changes:
%APIToken{}instead of a%User{}.api/v2/incidents/newendpoint.Questions
A few implementation notes and questions:
deletedandassignments, and theurlsfield. A few questions:deletedfield; I'm not sure why the endpoint isn't handling these values.assignments; I think this requires a larger change to thecast()usage inMedia.changeset/3. We also don't surface users' IDs so this would also require a smaller change to the in-platform API documentation on the Access page.urlslist validation fails silently—an otherwise well-formed API call with"url" => ["foobar"]results in a successful incident creation, but the URL is not archived or added as a piece of source material. Let me know if it's ok to add validation for this at theAPIV2Controllerlayer; I wasn't sure where to add this validation otherwise.A Note on Regressions
In the course of developing this endpoint, I encountered two previously undocumented bugs. We should fix these, but it's worth noting, both for review of this PR and for root causing of these bugs, that they were not caused by this PR. The two bugs are: