Limit 20 privs per purchase#993
Conversation
WalkthroughThe maximum allowed number of jurisdictions in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant AuthorizeNet
Client->>API: POST /purchase-privileges (selectedJurisdictions: [up to 20])
API->>API: Validate selectedJurisdictions.length <= 20
API->>AuthorizeNet: Create transaction (line items: privileges + fees)
AuthorizeNet-->>API: Transaction result
API-->>Client: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: in the compactconnect codebase, landonshumway-ia prefers to avoid extraneous unit tests when existin...Applied to files:
📚 Learning: in the compactconnect codebase, production active_compact_member_jurisdictions configurations are ve...Applied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
landonshumway-ia
left a comment
There was a problem hiding this comment.
This looks good. Do we also want to consider adding a check in the lambda itself should the API gateway request validation ever change? I'm fine shipping this as is if you don't think it's necessary
|
I'd normally include it in the Marshmallow schema as well but in the case of this endpoint, we aren't using one. We could add it, of course. |
|
Makes sense, this is one of the first endpoints I think I implemented for this project, so I just didn't consider adding the Marshmallow schema for it. We can leave it as is for now. |
|
@jlkravitz This is ready for your review. Thanks |
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen good to merge!
Requirements List
Testing List
Closes #714
Summary by CodeRabbit
New Features
Documentation