Annotations: BE#6478
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zoldar
left a comment
zoldar
left a comment
There was a problem hiding this comment.
LGTM, aside from the tests that probably need a slight update after the last round of changes.
|
|
||
| scope "/:domain/annotations", PlausibleWeb.Api.Internal, | ||
| private: %{allow_consolidated_views: true} do | ||
| pipeline :annotations_endpoints, |
There was a problem hiding this comment.
Pipelines are defined at the router root anyway (behind a macro). I'd either group it with other pipelines or - given it's now used only by a single controller - move the plug directly to the controller.
|
|
||
| test "an unparsable date string is rejected" do | ||
| changeset = | ||
| Annotation.changeset( |
There was a problem hiding this comment.
Side note - even for error cases, I'd recommend shaping the changeset and arguments in a way where all the other data are correct (for instance note and type aren't missing), so that the error case is tested in isolation from other errors.
2 participants
Changes
Adds the BE for annotations. It's heavily based on segments and in this PR, it piggybacks on SiteSegments billing feature for brevity. Accessible only with feature flag "annotations".
Tests
Changelog
Documentation
Dark mode