Feat/authorize net payment opaque data #847
Conversation
WalkthroughThis update refactors the payment processing flow for Authorize.Net to use opaque payment tokens instead of directly handling raw credit card data in the backend. It introduces new schema fields, updates API models, modifies test infrastructure, and ensures compliance by removing direct credit card data handling throughout the backend and test suites. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant AuthorizeNet
participant Backend
participant DynamoDB
Frontend->>AuthorizeNet: Send card data via Accept UI
AuthorizeNet-->>Frontend: Return opaque payment token (opaqueData)
Frontend->>Backend: Submit purchase request with opaqueData
Backend->>DynamoDB: Retrieve compact configuration & public keys
Backend->>AuthorizeNet: Process payment using opaqueData
AuthorizeNet-->>Backend: Return payment result
Backend-->>Frontend: Respond with transaction outcome
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
landonshumway-ia
force-pushed
the
feat/authorize-net-payment-opaque-data
branch
from
6a02498 to
d866bfb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py (1)
90-92: Simplify sandbox flag assignment
You can replace the ternary with a direct boolean expression to satisfy the static analysis hint and improve readability.- item['isSandbox'] = True if config.environment_name != 'prod' else False + item['isSandbox'] = config.environment_name != 'prod'🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 91-91: The if expression can be replaced with 'test'
(R1719)
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py (1)
321-332: Consider more robust response parsing.The current implementation assumes UTF-8 BOM in the response (
decode('utf-8-sig')). While this works, consider adding error handling for different response encodings.- response_data = json.loads(response.content.decode('utf-8-sig')) + try: + response_data = json.loads(response.content.decode('utf-8-sig')) + except UnicodeDecodeError: + # Fallback to standard UTF-8 if BOM is not present + response_data = json.loads(response.content.decode('utf-8'))backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py (1)
146-146: Consider adding a test helper method for config reset.While the current approach works, accessing the protected
_Config()constructor could be made cleaner by adding a test helper method in the config module itself.Consider adding a helper method in the
cc_common.configmodule:def reset_config_for_tests(): """Reset config singleton for test isolation.""" global config config = _Config()This would make the test code cleaner and avoid the need for the suppression comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js(2 hunks)backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/record.py(2 hunks)backend/compact-connect/lambdas/python/common/tests/resources/api/compact-configuration-response.json(0 hunks)backend/compact-connect/lambdas/python/purchases/handlers/privileges.py(1 hunks)backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py(4 hunks)backend/compact-connect/lambdas/python/purchases/purchase_client.py(10 hunks)backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_privilege_options.py(3 hunks)backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_purchase_privileges.py(2 hunks)backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py(8 hunks)backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py(10 hunks)backend/compact-connect/stacks/api_stack/v1_api/api.py(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/credentials.py(3 hunks)backend/compact-connect/stacks/transaction_monitoring_stack/transaction_history_processing_workflow.py(1 hunks)backend/compact-connect/tests/app/test_api/test_purchases_api.py(1 hunks)backend/compact-connect/tests/app/test_transaction_monitoring.py(1 hunks)backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/PURCHASE_PRIVILEGE_REQUEST_SCHEMA.json(1 hunks)backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py(2 hunks)backend/compact-connect/tests/smoke/config.py(1 hunks)backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py(7 hunks)backend/compact-connect/tests/smoke/smoke_tests_env_example.json(1 hunks)
💤 Files with no reviewable changes (1)
- backend/compact-connect/lambdas/python/common/tests/resources/api/compact-configuration-response.json
🧰 Additional context used
🧬 Code Graph Analysis (4)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py (1)
PaymentProcessorPublicFieldsSchema(43-51)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/record.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py (1)
PaymentProcessorPublicFieldsSchema(43-51)
backend/compact-connect/stacks/api_stack/v1_api/credentials.py (2)
backend/compact-connect/stacks/persistent_stack/__init__.py (1)
PersistentStack(39-566)backend/compact-connect/lambdas/python/common/cc_common/config.py (1)
compact_configuration_table(60-61)
backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py (2)
backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py (1)
process_settled_transactions(13-128)backend/compact-connect/lambdas/python/common/cc_common/config.py (1)
transaction_history_table(270-271)
🪛 Pylint (3.3.7)
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py
[refactor] 91-91: The if expression can be replaced with 'test'
(R1719)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py
[refactor] 43-43: Too few public methods (0/2)
(R0903)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py
[refactor] 134-134: Too many public methods (26/20)
(R0904)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
🔇 Additional comments (49)
backend/compact-connect/tests/smoke/config.py (1)
55-62: Sandbox Authorize.Net credentials properties added appropriately
The newsandbox_authorize_net_api_login_idandsandbox_authorize_net_transaction_keyproperties follow the existing pattern, sourcing sandbox credentials from environment variables for smoke tests.backend/compact-connect/tests/smoke/smoke_tests_env_example.json (1)
15-17: Include sandbox API credentials in example env file
AddingENVIRONMENT_NAME,SANDBOX_AUTHORIZE_NET_API_LOGIN_ID, andSANDBOX_AUTHORIZE_NET_TRANSACTION_KEYensures that smoke test environments can be configured with the placeholders needed by the new sandbox flows.backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js (3)
176-178: Authorize.Net domains whitelisted forscript-src
Thehttps://jstest.authorize.net/andhttps://js.authorize.net/entries align with the Accept UI requirements and mirror the existing recaptcha pattern.
183-185: Authorize.Net domains added toscript-src-elem
Consistent whitelisting across both inline and element script directives prevents CSP violations when loading Accept UI scripts.
222-224: Authorize.Net domains whitelisted forframe-src
Including the same domains in the frame source list is necessary for the Accept customer-hosted fields to render correctly.backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json (1)
42-42: Snapshot update references new Lambda version
The CloudFront distribution snapshot now points to the updatedCSPFunctionCurrentVersion—this matches the deployed Lambda changes and keeps tests in sync.backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json (1)
42-42: LGTM! Lambda function version updated for CSP enhancement.The Lambda function reference change aligns with the CSP header updates to support Authorize.net domains for secure payment processing.
backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json (1)
42-42: LGTM! Production environment aligned with Beta.The Lambda function version update ensures consistent CSP behavior across environments for Authorize.net integration.
backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js (1)
89-90:✅ Verification successful
Verify Authorize.net domain legitimacy and CSP directive appropriateness.
The additions of
https://jstest.authorize.net/andhttps://js.authorize.net/to CSP directives support the new payment nonce workflow. Ensure these are the correct official Authorize.net domains and that both test and production domains are appropriately included.Also applies to: 96-97, 135-136
🌐 Web query:
What are the official Authorize.Net JavaScript SDK domains for production and testing environments?💡 Result:
When integrating the Authorize.Net JavaScript SDK, it's essential to use the correct domains for your environment to ensure proper functionality. The domains differ between the sandbox (testing) and production environments.
Sandbox (Testing) Environment:
- Accept.js Library: For testing purposes, include the Accept.js library from the sandbox domain:
<script type="text/javascript" src="https://jstest.authorize.net/v1/Accept.js" charset="utf-8"></script>(community.developer.cybersource.com)
Production Environment:
- Accept.js Library: For live transactions, use the production domain:
<script type="text/javascript" src="https://js.authorize.net/v1/Accept.js" charset="utf-8"></script>(community.developer.cybersource.com)
Ensure that your integration points to the appropriate domain based on your environment to maintain secure and accurate transaction processing.
Citations:
- 1: https://community.developer.cybersource.com/t5/Integration-and-Testing/Feedback-on-Accept-js-implementation/td-p/57170?utm_source=openai
- 2: https://community.developer.cybersource.com/t5/Integration-and-Testing/Feedback-on-Accept-js-implementation/td-p/57170?utm_source=openai
Authorize.Net SDK domains verified for CSP tests
Confirmed that
https://jstest.authorize.net(sandbox) andhttps://js.authorize.net(production) are the official Accept.js domains. They’re correctly included in the CSP test at:
- script-src (lines 89–90)
- script-src-elem (lines 96–97)
- frame-src (lines 135–136)
No further changes required.
backend/compact-connect/stacks/api_stack/v1_api/api.py (1)
136-136: LGTM! Proper integration of persistent stack for credential management.The addition of the
persistent_stackparameter to theCredentialsconstructor enables access to the compact configuration table, which is necessary for storing public payment processor credentials as outlined in the PR objectives.backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py (2)
13-13: LGTM! Correct import for payment processor public fields schema.The import aligns with the new payment processing architecture that exposes public credentials to the frontend.
32-33:✅ Verification successful
Verify schema consistency and field requirements.
The new fields look correctly implemented for the payment processor integration:
paymentProcessorPublicFields: Nested schema containing public credentials for frontend payment form generationisSandbox: Boolean indicating sandbox environment statusBoth fields are marked as required, which is appropriate since the frontend needs these values for payment processing.
Let me verify that the
PaymentProcessorPublicFieldsSchemacontains the expected fields:
🏁 Script executed:
#!/bin/bash # Description: Verify the PaymentProcessorPublicFieldsSchema definition matches expected structure # Search for the PaymentProcessorPublicFieldsSchema definition ast-grep --pattern $'class PaymentProcessorPublicFieldsSchema($_): $$$'Length of output: 1260
Schema Verified
The
PaymentProcessorPublicFieldsSchemainbackend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.pydefines bothpublicClientKeyandapiLoginIdasString(required=True, allow_none=False), which matches the integration requirements. ThepaymentProcessorPublicFieldsandisSandboxfields inapi.pyare correctly marked as required and non-nullable. No changes needed.backend/compact-connect/tests/app/test_transaction_monitoring.py (1)
153-168: LGTM! Comprehensive test for workflow initialization changes.The new test method correctly verifies that the transaction monitoring workflow's initialize state produces the expected JSON structure with:
- Proper extraction of
scheduledTimefrom EventBridge events- Correct initialization of compact and processedBatchIds
- Appropriate result path configuration
The test follows the established pattern in the file and uses the helper method for token normalization.
backend/compact-connect/stacks/transaction_monitoring_stack/transaction_history_processing_workflow.py (1)
100-110: LGTM! Improved workflow initialization for replay-ability.The changes correctly implement transaction processing replay-ability by:
- Extracting scheduled time:
'scheduledTime.$': '$.time'extracts the EventBridge event time- Simplified initialization: Using
Passwithparametersis cleaner than the previousResult.from_objectapproach- Proper payload structure: Setting
result_path='$.Payload'maintains consistency with downstream processingThis enables deterministic transaction collection windows for replay scenarios.
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/record.py (1)
11-11: LGTM! Clean schema enhancement for payment processor integration.The addition of the
paymentProcessorPublicFieldsfield is well-implemented with:
- Proper import of the nested schema
- Clear documentation explaining the field's purpose
- Appropriate validation (
required=False, allow_none=False)- Good alignment with the security enhancement to use payment nonces
Also applies to: 41-43
backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json (1)
5-5: Consistent CSP updates across environments.The CSP header changes are identical to the BetaFrontend snapshot, ensuring consistent Authorize.Net domain allowlisting across all environments.
backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json (1)
5-5: Production environment CSP updates complete the consistent deployment.The CSP header changes match the other environment snapshots, completing the consistent Authorize.Net domain allowlisting across Beta, Test, and Production environments.
backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_privilege_options.py (4)
10-11: LGTM: Well-defined test constants.The mock constants are clearly named and will be consistently used throughout the test file.
17-24: LGTM: Proper test setup with mock payment processor fields.The test setup correctly includes the new payment processor public fields in the compact configuration, which aligns with the broader payment processor integration changes.
49-65: LGTM: Test assertion correctly validates jurisdiction option structure.The hardcoded expected structure matches the test setup and includes all necessary jurisdiction fields.
83-99: LGTM: Comprehensive compact option validation.The test assertion correctly validates the new payment processor public fields and sandbox flag alongside existing compact configuration fields.
backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py (3)
6-6: LGTM: Proper import of configuration.The import of config module enables access to sandbox credentials for testing.
274-332: LGTM: Well-implemented smoke test for payment processor credentials.The test function properly:
- Creates a test compact admin user with appropriate permissions
- Uses sandbox credentials from configuration
- Includes proper error handling and validation
- Cleans up test resources in the finally block
- Uses appropriate timeout for external API calls
336-339: Verify if commented tests are intentional.The existing smoke tests are commented out, leaving only the new payment processor test active. This might be intentional for focused testing during development.
Is this temporary for development/testing purposes, or should the other smoke tests remain active?
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py (2)
12-13: LGTM: Well-defined payment processor type enum.The enum provides a consistent way to reference the Authorize.Net payment processor type across the codebase.
43-51: LGTM: Appropriate schema for payment processor public fields.The schema correctly defines the public fields needed for client-side payment form generation. The static analysis warning about too few public methods is a false positive - data schemas don't need multiple public methods.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 43-43: Too few public methods (0/2)
(R0903)
backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py (1)
189-215: LGTM! Well-implemented method for storing Authorize.Net public credentials.The implementation follows best practices:
- Proper validation by checking compact configuration existence before update
- Uses UPDATE with SET to avoid overwriting other configuration fields
- Includes appropriate logging for debugging
- Clear documentation and type hints
backend/compact-connect/stacks/api_stack/v1_api/credentials.py (2)
26-44: LGTM! Proper integration with persistent stack for compact configuration access.The constructor changes correctly:
- Accept the
persistent_stackparameter with proper typing- Set the environment variable from the persistent stack's table name
- Pass the parameter to the helper method for permission configuration
46-57: LGTM! Correct permission configuration for compact configuration table access.The method properly grants read/write permissions to the Lambda handler for accessing the compact configuration table, enabling storage of payment processor public credentials.
backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py (3)
28-37: LGTM! Proper parameter handling for scheduledTime.The scheduledTime parameter is correctly extracted from the event and the documentation is updated appropriately.
42-47: LGTM! Improved replay-ability with scheduled time anchoring.The time calculation correctly uses the scheduled time instead of current time, enabling consistent transaction retrieval windows for replay scenarios. The use of
datetime.fromisoformatproperly handles the ISO format from EventBridge.
87-87: LGTM! Proper preservation of scheduledTime for workflow continuity.Preserving the scheduledTime in the response ensures consistent time anchoring across multiple workflow iterations.
backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py (4)
28-28: LGTM! Proper mock constant for scheduled time testing.The
MOCK_SCHEDULED_TIMEconstant uses the correct ISO format expected from EventBridge.
141-150: LGTM! Consistent event structure updates for scheduledTime.Both event creation methods properly include the
scheduledTimeparameter using the mock constant, ensuring test consistency.
200-208: LGTM! Comprehensive test assertion updates for scheduledTime.All test assertions are properly updated to expect the
scheduledTimefield in responses, ensuring complete test coverage of the new functionality.Also applies to: 350-350, 418-418, 441-441
299-334: LGTM! Excellent addition of idempotency test.The new test properly verifies that repeated invocations with identical events don't create duplicate transaction records, which is crucial for replay-ability scenarios. The test setup and assertions are appropriate.
backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_purchase_privileges.py (1)
36-38: LGTM! Test data correctly updated for opaque payment data.The test constants and request body structure have been properly updated to use opaque payment data instead of raw card details, aligning with the new payment processing approach.
Also applies to: 65-65
backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py (1)
49-51: Well-structured test enhancements for payment processor integration.The test correctly:
- Ensures payment processor credentials are uploaded before testing
- Verifies the
isSandboxflag is set appropriately- Validates that public payment fields are properly exposed in the API response
Also applies to: 74-74, 88-104
backend/compact-connect/lambdas/python/purchases/purchase_client.py (2)
323-330: Verify that removing billing address doesn't impact fraud prevention.The implementation correctly uses opaque data, but billing address has been removed. Please ensure that:
- Authorize.Net's Accept UI captures billing address on the frontend if needed for AVS checks
- Your fraud prevention rules don't require billing address data
970-987: Excellent error handling for credential storage.The implementation properly handles the edge case where compact configuration doesn't exist, providing clear guidance to configure fees before uploading credentials.
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py (9)
3-3: LGTM! Imports properly added for AWS mocking.The addition of
os,boto3, andmoto.mock_awsimports is appropriate for the new DynamoDB table setup in tests.Also applies to: 7-10
58-64: LGTM! Mock constants properly defined for opaque payment data.The new constants
MOCK_DATA_DESCRIPTORandMOCK_DATA_VALUEcorrectly represent the opaque payment token structure, andMOCK_PUBLIC_CLIENT_KEYis appropriately defined for the merchant details response.
85-85: LGTM! Order information correctly updated to use opaque data.The function now returns the proper structure with
opaqueDatacontainingdataDescriptoranddataValue, aligning with the PR's security improvements.
133-166: Test setup properly implements DynamoDB mocking with appropriate cleanup.The setUp method correctly creates a DynamoDB table for compact configuration and ensures cleanup with
addCleanup. This prevents test pollution and aligns with the new requirement for compact configuration presence.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 134-134: Too many public methods (26/20)
(R0904)
340-342: LGTM! Assertions correctly updated for opaque payment data.The test properly verifies that the opaque data fields (
dataDescriptoranddataValue) are being passed to the Authorize.Net SDK instead of raw card data, implementing the security improvements.
710-712: LGTM! Helper method properly sets up compact configuration.The
_when_compact_configuration_existsmethod correctly uses the test data generator to create the necessary compact configuration in DynamoDB.
714-722: LGTM! Response correctly includes public client key.The mock response properly includes the
publicClientKeyfield, which is necessary for the frontend to generate the payment collection form using Authorize.Net's Accept UI solution.
824-853: Excellent test coverage for missing compact configuration edge case.The test properly validates that the system throws a clear, user-friendly error message when attempting to store payment credentials without a compact configuration. The error message provides actionable guidance to configure the compact fee values first.
134-134: Skip pylint warning about too many public methods.Having 26 test methods in a test class is acceptable. Each test method focuses on a specific scenario, and splitting them across multiple classes would reduce code organization and readability.
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 134-134: Too many public methods (26/20)
(R0904)
backend/compact-connect/tests/app/test_api/test_purchases_api.py
Outdated
Show resolved
Hide resolved
...esources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json
Outdated
Show resolved
Hide resolved
backend/compact-connect/tests/resources/snapshots/PURCHASE_PRIVILEGE_REQUEST_SCHEMA.json
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/purchases/purchase_client.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py
Show resolved
Hide resolved
landonshumway-ia
force-pushed
the
feat/authorize-net-payment-opaque-data
branch
from
8d85cec to
87ff589
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py (1)
90-91: Simplify the boolean assignment.The logic is correct and aligns with the PR objective of exposing sandbox mode to the frontend. However, the conditional expression can be simplified.
Apply this diff to simplify the boolean assignment:
- # we determine at run-time if the payment processor is running in sandbox mode - item['isSandbox'] = True if config.environment_name != 'prod' else False + # we determine at run-time if the payment processor is running in sandbox mode + item['isSandbox'] = config.environment_name != 'prod'🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 91-91: The if expression can be replaced with 'test'
(R1719)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js(2 hunks)backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/record.py(2 hunks)backend/compact-connect/lambdas/python/common/tests/resources/api/compact-configuration-response.json(0 hunks)backend/compact-connect/lambdas/python/purchases/handlers/privileges.py(1 hunks)backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py(4 hunks)backend/compact-connect/lambdas/python/purchases/purchase_client.py(10 hunks)backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_privilege_options.py(3 hunks)backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_purchase_privileges.py(2 hunks)backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py(8 hunks)backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py(10 hunks)backend/compact-connect/stacks/api_stack/v1_api/api.py(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py(1 hunks)backend/compact-connect/stacks/api_stack/v1_api/credentials.py(3 hunks)backend/compact-connect/stacks/transaction_monitoring_stack/transaction_history_processing_workflow.py(1 hunks)backend/compact-connect/tests/app/test_transaction_monitoring.py(1 hunks)backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/PURCHASE_PRIVILEGE_REQUEST_SCHEMA.json(1 hunks)backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json(1 hunks)backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json(1 hunks)backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py(2 hunks)backend/compact-connect/tests/smoke/config.py(1 hunks)backend/compact-connect/tests/smoke/load_test_privilege_purchasing.py(5 hunks)backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py(5 hunks)backend/compact-connect/tests/smoke/smoke_common.py(2 hunks)backend/compact-connect/tests/smoke/smoke_tests_env_example.json(1 hunks)
💤 Files with no reviewable changes (1)
- backend/compact-connect/lambdas/python/common/tests/resources/api/compact-configuration-response.json
✅ Files skipped from review due to trivial changes (1)
- backend/compact-connect/tests/smoke/config.py
🚧 Files skipped from review as they are similar to previous changes (27)
- backend/compact-connect/tests/smoke/smoke_tests_env_example.json
- backend/compact-connect/lambdas/nodejs/cloudfront-csp/index.js
- backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json
- backend/compact-connect/tests/app/test_transaction_monitoring.py
- backend/compact-connect/lambdas/nodejs/cloudfront-csp/test/index.test.js
- backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json
- backend/compact-connect/tests/resources/snapshots/BetaFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/record.py
- backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION.json
- backend/compact-connect/tests/resources/snapshots/TestFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json
- backend/compact-connect/tests/resources/snapshots/ProdFrontend-FrontendDeploymentStack-UI_DISTRIBUTION_LAMBDA_FUNCTION.json
- backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py
- backend/compact-connect/stacks/api_stack/v1_api/api.py
- backend/compact-connect/tests/resources/snapshots/PURCHASE_PRIVILEGE_REQUEST_SCHEMA.json
- backend/compact-connect/stacks/api_stack/v1_api/credentials.py
- backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_privilege_options.py
- backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py
- backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_purchase_privileges.py
- backend/compact-connect/stacks/transaction_monitoring_stack/transaction_history_processing_workflow.py
- backend/compact-connect/lambdas/python/purchases/handlers/transaction_history.py
- backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py
- backend/compact-connect/tests/smoke/load_test_privilege_purchasing.py
- backend/compact-connect/tests/smoke/smoke_common.py
- backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py
- backend/compact-connect/stacks/api_stack/v1_api/api_model.py
- backend/compact-connect/tests/smoke/purchasing_privileges_smoke_tests.py
- backend/compact-connect/lambdas/python/purchases/purchase_client.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
CCEnum(180-190)
🪛 Pylint (3.3.7)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py
[refactor] 134-134: Too many public methods (26/20)
(R0904)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py
[refactor] 43-43: Too few public methods (0/2)
(R0903)
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py
[refactor] 91-91: The if expression can be replaced with 'test'
(R1719)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: TestPython
🔇 Additional comments (7)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/common.py (2)
12-14: LGTM! Well-structured enum addition.The
PaymentProcessorTypeenum properly extendsCCEnumand follows the established naming conventions. This supports the transition to Authorize.Net opaque data handling.
43-51: LGTM! Essential schema for secure payment processing.The
PaymentProcessorPublicFieldsSchemacorrectly defines the required fields (publicClientKeyandapiLoginId) needed for client-side payment form generation. This is a key component of the security improvement that eliminates backend handling of raw card data.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 43-43: Too few public methods (0/2)
(R0903)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py (5)
58-64: LGTM! Proper mock data for secure payment testing.The addition of mock constants for opaque data (
MOCK_DATA_DESCRIPTOR,MOCK_DATA_VALUE) and public client key properly supports testing the new secure payment flow without exposing raw card data.
133-166: LGTM! Comprehensive AWS mocking infrastructure.The addition of
@mock_awsdecorator and DynamoDB table setup/teardown provides proper test isolation and supports testing the new compact configuration validation logic.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 134-134: Too many public methods (26/20)
(R0904)
85-85: LGTM! Secure payment data for testing.The updated
_generate_default_order_information()method now returns opaque data instead of raw card details, which aligns perfectly with the PR objective of enhancing payment security.
340-342: LGTM! Updated assertions for opaque data validation.The test correctly validates that opaque data fields (
dataDescriptoranddataValue) are properly passed to the Authorize.Net SDK instead of raw card data, ensuring the security improvements are working as expected.
824-853: LGTM! Essential test coverage for validation logic.This new test ensures that credential validation fails when compact configuration doesn't exist, providing good coverage for the new validation workflow and clear error messaging to users.
Previously, if a transaction history collection job failed to process for whatever reason (such as a service outage), it was incredibly difficult to run the processing for that specific time period again. This change leverages the fact that event bridge sends in the timestamp of the time in which the scheduled task was triggered. By going off of this time, rather than generating the timestamp in the lambda, developers can simply rerun the failed step function execution, and it will process all the transactions for that same time period. Due to the idempotent nature of the transaction pk/sk design, we will never see duplicate transaction records for the same transaction ids.
landonshumway-ia
force-pushed
the
feat/authorize-net-payment-opaque-data
branch
from
f838ce8 to
3a12b3c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (4)
179-181: Prefer nullish-coalescing over logical OR for precise fallback
|| nullwill coerce falsy objects (e.g.,{} as PaymentSdkConfig) tonull, masking data that is merely “empty.”
Using??keeps the intent clear and only falls back when the value isnull/undefined.-get currentCompactPaymentSdkConfig(): PaymentSdkConfig | null { - return this.currentCompact?.fees?.paymentSdkConfig || null; +get currentCompactPaymentSdkConfig(): PaymentSdkConfig | null { + return this.currentCompact?.fees?.paymentSdkConfig ?? null; }
280-285: Add explicit parameter & return types toshouldUseMilitaryRateThe helper currently accepts
any, weakening type-safety and IntelliSense.
A minimal structural type is enough to clarify intent and catch future regressions.-shouldUseMilitaryRate(feeConfig) { +shouldUseMilitaryRate(feeConfig: { militaryRate?: number }): boolean { const { militaryRate } = feeConfig || {}; const hasMilitaryRate = Boolean(militaryRate || militaryRate === 0); return Boolean(hasMilitaryRate && this.licensee?.isMilitaryStatusActive()); }
300-318: NarrowopaqueDataparameter type to match Accept UI contract
opaqueDataarrives from Authorize.Net and has a well-defined shape ({ dataDescriptor: string; dataValue: string }).
Keeping it asobjectforfeits type safety and makes downstream mistakes harder to spot.-import type { AcceptUiResponse } from '@components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.vue'; +import type { + AcceptUiResponse, + OpaqueData /* <- interface exported from Accept UI component */ +} from '@components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.vue'; ... -async handleSubmitOverride(opaqueData: object): Promise<void> { +async handleSubmitOverride(opaqueData: OpaqueData): Promise<void> {
332-335: Scroll error message into view after backend failureIn the backend-error branch the form message is set but not scrolled into view, unlike
acceptUiErrorResponse.
This small UX tweak keeps feedback consistent.} else if (purchaseServerEvent?.message) { this.isFormError = true; this.formErrorMessage = purchaseServerEvent?.message; + await nextTick(); + document + .getElementById('button-row') + ?.scrollIntoView({ behavior: 'smooth', block: 'center' }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
webroot/src/components/PrivilegePurchaseAttestation/PrivilegePurchaseAttestation.ts(3 hunks)webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts(7 hunks)webroot/src/locales/en.json(1 hunks)webroot/src/locales/es.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- webroot/src/components/PrivilegePurchaseAttestation/PrivilegePurchaseAttestation.ts
- webroot/src/locales/en.json
- webroot/src/locales/es.json
🧰 Additional context used
🧠 Learnings (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (1)
Learnt from: jsandoval81
PR: csg-org/CompactConnect#847
File: webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts:195-197
Timestamp: 2025-06-11T19:43:08.839Z
Learning: For the MVP phase, the team prefers to surface the raw "undefined" (or similar) when the compact commission fee is missing, rather than showing a misleading fallback such as "0.00"; the server-side validation will catch invalid values.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: TestPython
- GitHub Check: CheckWebroot
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (1)
314-346: 🛠️ Refactor suggestionAdd a catch-all error branch for unexpected server responses
If
purchaseServerEventisnull/undefined, or the backend changes the success message, the user receives no feedback and the form stays in a loading state. Guard against this by adding a default error handler and relying on a more robust success signal than a free-text string comparison.- } else if (purchaseServerEvent?.message) { + } else if (purchaseServerEvent?.message) { this.isFormError = true; this.formErrorMessage = purchaseServerEvent?.message; await this.scrollIntoView('button-row'); + } else { + // Fallback for network / unexpected errors + this.isFormError = true; + this.formErrorMessage = this.$t('common.unexpectedError'); + await this.scrollIntoView('button-row'); }Consider having the backend return a boolean
successflag (or HTTP status) to avoid brittle string comparisons.
🧹 Nitpick comments (2)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (2)
280-285: Type thefeeConfigparameter for better IntelliSense and safetyExplicitly typing the argument avoids
any, documents expected shape, and surfaces mistakes during refactors.- shouldUseMilitaryRate(feeConfig) { + shouldUseMilitaryRate( + feeConfig: { militaryRate?: number; baseRate?: number } | null | undefined + ): boolean {
209-218: Reuse the existingprivilegeCountgetter to reduce duplication
creditCardFeesTotalmanually recalculates the privilege count. Leveraging the computed property keeps the logic in one place.- const numPrivileges = this.selectedStatePurchaseDataList?.length || 0; + const numPrivileges = this.privilegeCount;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (1)
Learnt from: jsandoval81
PR: csg-org/CompactConnect#847
File: webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts:195-197
Timestamp: 2025-06-11T19:43:08.839Z
Learning: For the MVP phase, the team prefers to surface the raw "undefined" (or similar) when the compact commission fee is missing, rather than showing a misleading fallback such as "0.00"; the server-side validation will catch invalid values.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: TestPython
- GitHub Check: CheckWebroot
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts
Show resolved
Hide resolved
If a state has not been marked as live, we need to ensure it is not an option for practitioners to purchase privileges from.
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.less
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.vue
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts
Outdated
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (1)
306-346: 🛠️ Refactor suggestionGuarantee spinner cleanup and capture network errors
startFormLoading()is called before the async dispatch, butendFormLoading()is only reached on the success path. If the action throws (network error, unhandled 5xx, etc.) the loading state never resets and the user is stuck.- this.startFormLoading(); - ... - const purchaseServerEvent = await this.$store.dispatch('user/postPrivilegePurchases', serverData); - this.endFormLoading(); + this.startFormLoading(); + let purchaseServerEvent; + try { + purchaseServerEvent = await this.$store.dispatch( + 'user/postPrivilegePurchases', + serverData + ); + } catch (err) { + purchaseServerEvent = { message: err?.message ?? 'Unexpected error' }; + } finally { + this.endFormLoading(); + }Placing
endFormLoading()infinallyensures the UI recovers regardless of how the promise resolves.
♻️ Duplicate comments (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (1)
179-185:||still discards legitimate zero-value commission feesUsing the logical-OR fallback converts a valid
0fee intonull, which in turn causestotalCompactCommissionFee(lines 191-199) to ignore the fee entirely. The same concern was raised on an earlier commit and left unresolved for domain reasons, so I’m just flagging for visibility.
🧹 Nitpick comments (2)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (2)
280-285: Add an explicit type forfeeConfig
shouldUseMilitaryRatereceives an untypedfeeConfig, which weakens type-safety and IntelliSense. Declare it as the concrete fee-config interface to surface missing properties at compile time.-shouldUseMilitaryRate(feeConfig): boolean { +shouldUseMilitaryRate(feeConfig: PrivilegePurchaseOption['fees'][string]): boolean {
297-304: Retrieve the element afternextTickto avoid stale refs
scrollIntoViewfetches the DOM node beforeawait nextTick(), so updates triggered earlier in the call stack might replace the element, leaving you with a stale reference. Move the query after the tick.-const formMessageElement = document.getElementById(id); - -if (formMessageElement?.scrollIntoView) { - await nextTick(); - formMessageElement?.scrollIntoView({ behavior: 'smooth', block: 'center' }); -} +await nextTick(); +const el = document.getElementById(id); +el?.scrollIntoView({ behavior: 'smooth', block: 'center' });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts(8 hunks)webroot/src/locales/es.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- webroot/src/locales/es.json
🧰 Additional context used
🧠 Learnings (1)
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts (1)
Learnt from: jsandoval81
PR: csg-org/CompactConnect#847
File: webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts:195-197
Timestamp: 2025-06-11T19:43:08.839Z
Learning: For the MVP phase, the team prefers to surface the raw "undefined" (or similar) when the compact commission fee is missing, rather than showing a misleading fallback such as "0.00"; the server-side validation will catch invalid values.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestPython
|
@jlkravitz This is ready for your review. FYI the unresolved comments from @rmolinares above are actually resolved, but neither him nor I have the ability to mark them resolved. If you're able to then it's a green light to do so. |
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/record.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py
Outdated
Show resolved
Hide resolved
webroot/src/components/PrivilegePurchaseFinalize/PrivilegePurchaseFinalize.ts
Show resolved
Hide resolved
backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py
Outdated
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py
Show resolved
Hide resolved
...ct-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py
Outdated
Show resolved
Hide resolved
...ct-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Joshua Kravitz <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py (1)
162-166: Moto waiter still unsupported – this teardown will crash
Moto does not implementget_waiter('table_not_exists'), so line 165 raisesUnknownOperationError, breaking test cleanup.
Same issue was flagged previously and is still unresolved.- waiter = self._compact_configuration_table.meta.client.get_waiter('table_not_exists') - waiter.wait(TableName=self._compact_configuration_table.name) + # Moto has no waiter implementation; a short sleep is enough + import time + time.sleep(0.1)
🧹 Nitpick comments (1)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py (1)
841-851: Assertion could be simplified & made less fragile
CCInvalidRequestExceptionalready stringifies to the message; asserting onstr(context.exception)avoids reaching into a.messageattribute (which isn’t standard on Python exceptions).- str(context.exception.message), + str(context.exception),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py(8 hunks)backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py(10 hunks)backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/compact-connect/tests/smoke/compact_configuration_smoke_tests.py
- backend/compact-connect/lambdas/python/purchases/tests/function/test_handlers/test_transaction_history.py
🧰 Additional context used
🪛 Pylint (3.3.7)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py
[refactor] 135-135: Too many public methods (26/20)
(R0904)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: CheckWebroot
- GitHub Check: TestPython
🔇 Additional comments (3)
backend/compact-connect/lambdas/python/purchases/tests/unit/test_purchase_client.py (3)
151-160: Possible hidden dependency on env varCOMPACT_CONFIGURATION_TABLE_NAME
create_compact_configuration_table()extracts the table name fromos.environ.
If a caller (orTstLambdas) forgets to prime this variable, tests will explode withKeyError.
Consider supplying a default (os.getenv('…', 'compact-configuration')) or asserting its presence duringsetUpto fail fast with a clearer error.
711-713: Static access toTestDataGeneratormay be brittle
self.test_data_generatoris set to the classTestDataGenerator, then called without instantiation.
Ifput_default_compact_configuration_in_configuration_tableis an instance method, this will raiseTypeError.
Double-check that the helper is a@staticmethod/@classmethod, or instantiate:self.test_data_generator = TestDataGenerator() # if instance method
341-344: Opaque-data assertions look goodNew assertions confirm that the SDK receives the expected
dataDescriptoranddataValue; this tightens coverage around the tokenised payment flow.
webroot/src/components/PrivilegePurchaseAcceptUI/PrivilegePurchaseAcceptUI.ts
Show resolved
Hide resolved
Previously, our initial design for processing payment information when purchasing privileges involved sending card data directly to the backend, which was then used to call Authorize.net to make a transaction. In order to improve the security of the system, we are updating the front end to send the payment card information to Authorize.net directly using their Accept UI solution, and then sending the payment nonce token to the backend instead. This updates the backend to support that pattern.
As part of supporting this, the frontend will now need the api login id and the public client key to generate the payment collection form. This updates the system so that when a compact admin uploads their payment processing creds into the system, the public key is stored in the compact configuration object, which will then be accessible by the frontend when practitioners go to purchase privileges.
Closes #794
Closes #793
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Chores