Skip to content

MPT-17655 Show New account instructions heading on purchase wizard#228

Merged
ruben-sebrango merged 1 commit intomainfrom
MPT-17655-show-new-account-instructions-heading-on-purchase-wizard
Feb 5, 2026
Merged

MPT-17655 Show New account instructions heading on purchase wizard#228
ruben-sebrango merged 1 commit intomainfrom
MPT-17655-show-new-account-instructions-heading-on-purchase-wizard

Conversation

@alephsur
Copy link
Contributor

@alephsur alephsur commented Feb 4, 2026

Closes MPT-17655

  • Added new order parameters: NEW_ACCOUNT_INSTRUCTIONS, TECHNICAL_CONTACT_INFO, CONNECT_AWS_BILLING_ACCOUNT to OrderParametersEnum
  • Updated account-type parameter visibility/configuration (ACCOUNT_TYPE_CONFIG) to include new parameters and adjust visible/reset parameter sets for NEW_AWS_ENVIRONMENT and EXISTING_AWS_ENVIRONMENT
  • Introduced validation to enforce new-account constraints: _validate_new_account_constraints() returns AWS002 when NEW_ACCOUNT_INSTRUCTIONS is visible in disallowed contexts
  • Refactored validation internals: renamed add_default_lines -> _add_default_lines and added helpers _is_parameter_visible and _apply_account_type_constraints; validate_order orchestrates reset, constraint validation, constraints application, and default-line population
  • Tests and fixtures updated: added NEW_ACCOUNT_INSTRUCTIONS to test order parameters factory and refactored validation tests to use set_order_parameter_constraints() and updated expectations
  • Minor tooling change: updated pyproject.toml per-file-ignores for tests/flows/validation/test_base.py (added WPS210)

@alephsur alephsur requested a review from a team as a code owner February 4, 2026 16:15
@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

This PR adds three new order-parameter enum members and refactors the validation flow: it makes several helper functions internal, introduces new constraint-checking for new-account parameters (raising AWS002 when violated), updates ACCOUNT_TYPE_CONFIG mappings, and adapts tests to use the new parameter-constraint API.

Changes

Cohort / File(s) Summary
Config
pyproject.toml
Added per-file-ignore WPS210 for the validation test file.
Enum Constants
swo_aws_extension/constants.py
Added OrderParametersEnum members: NEW_ACCOUNT_INSTRUCTIONS, TECHNICAL_CONTACT_INFO, CONNECT_AWS_BILLING_ACCOUNT.
Validation flow
swo_aws_extension/flows/validation/base.py
Refactored validation orchestration: added imports for parameter helpers, updated ACCOUNT_TYPE_CONFIG visible/reset mappings, renamed add_default_lines_add_default_lines, added _is_parameter_visible and _validate_new_account_constraints, and changed validate_order order to reset errors, validate new-account constraints (emit AWS002), apply account-type constraints, then add default lines.
Tests — fixtures
tests/conftest.py
Appended a new order_parameters_factory entry for NEW_ACCOUNT_INSTRUCTIONS (heading-type parameter).
Tests — validation
tests/flows/validation/test_base.py
Reworked tests to remove direct calls to removed public helpers, use set_order_parameter_constraints and validate_order, and updated expectations (including AWS002 behavior and new parameter visibility checks).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Jira Issue Key In Title ✅ Passed The PR title contains exactly one Jira issue key in the correct MPT-XXXX format (MPT-17655).
Test Coverage Required ✅ Passed The PR modifies 2 code files and includes changes in 2 test files alongside the code changes, satisfying test coverage requirements.
Single Commit Required ✅ Passed The PR contains exactly one commit with all described changes, keeping git history clean and trackable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • MPT-17655: Cannot read properties of undefined (reading 'map')

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@swo_aws_extension/flows/validation/base.py`:
- Around line 41-45: The reset_params list for EXISTING_AWS_ENVIRONMENT contains
a duplicate OrderParametersEnum.ORDER_ACCOUNT_NAME.value; update the second
duplicate to OrderParametersEnum.ORDER_ACCOUNT_EMAIL.value so the reset_params
mirror the pattern used in NEW_AWS_ENVIRONMENT — locate the
EXISTING_AWS_ENVIRONMENT definition and replace the duplicate ORDER_ACCOUNT_NAME
entry with ORDER_ACCOUNT_EMAIL in the reset_params array.

In `@tests/flows/validation/Untitled`:
- Line 1: The file "Untitled" contains an orphaned placeholder function name
test_validate_order_orchestrates_all_steps with no code; remove this file from
the PR (delete the file) and ensure there are no remaining imports or references
to test_validate_order_orchestrates_all_steps elsewhere (confirm real tests live
in test_base.py) before committing the change.
🧹 Nitpick comments (2)
tests/flows/validation/test_base.py (2)

40-42: Unused variable assignment.

The variable new_account_instructions_param is assigned on lines 40-42 but never used before being reassigned on lines 46-48 after calling validate_order. This assignment can be removed.

♻️ Proposed fix
     order = set_order_parameter_constraints(
         order,
         OrderParametersEnum.NEW_ACCOUNT_INSTRUCTIONS.value,
         constraints={"hidden": True, "required": False, "readonly": False},
     )
-    new_account_instructions_param = get_ordering_parameter(
-        OrderParametersEnum.NEW_ACCOUNT_INSTRUCTIONS.value, order
-    )

     result = validate_order(mock_client, order)

96-98: Unused mocker argument.

The mocker fixture is declared as a parameter but not used in this test function, as flagged by static analysis (ARG001).

♻️ Proposed fix
 def test_validate_order_returns_error_when_new_account_instructions_visible(
-    order_factory, order_parameters_factory, mocker
+    order_factory, order_parameters_factory
 ):

@alephsur alephsur force-pushed the MPT-17655-show-new-account-instructions-heading-on-purchase-wizard branch from dd7c198 to 7592eb4 Compare February 4, 2026 16:33
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@tests/flows/validation/test_base.py`:
- Around line 96-98: The test function
test_validate_order_returns_error_when_new_account_instructions_visible includes
an unused fixture parameter `mocker`; remove the `mocker` parameter from the
function signature so the test signature matches its usage and avoids
unused-argument warnings (locate and edit the def for
test_validate_order_returns_error_when_new_account_instructions_visible to drop
the `mocker` parameter).
- Around line 40-42: Remove the dead assignment to
new_account_instructions_param: the call to
get_ordering_parameter(OrderParametersEnum.NEW_ACCOUNT_INSTRUCTIONS.value,
order) is unnecessary because new_account_instructions_param is never used
before being overwritten after validate_order; delete that assignment (the
statement that sets new_account_instructions_param) to eliminate the unused
variable and any redundant call.
🧹 Nitpick comments (1)
pyproject.toml (1)

129-129: Potentially redundant per-file-ignore entry.

The WPS210 ignore is already covered by the broader rule on line 110 (tests/**/*.py: WPS202 WPS204 WPS210 ...). The tests/flows/validation/test_base.py entry only needs to add WPS118 since that's not in the broader rule.

Consider simplifying to:

-  "tests/flows/validation/test_base.py: WPS118 WPS210",
+  "tests/flows/validation/test_base.py: WPS118",

@alephsur alephsur force-pushed the MPT-17655-show-new-account-instructions-heading-on-purchase-wizard branch from 7592eb4 to 7d51144 Compare February 4, 2026 17:07
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 4, 2026

@ruben-sebrango ruben-sebrango merged commit fde0042 into main Feb 5, 2026
11 checks passed
@ruben-sebrango ruben-sebrango deleted the MPT-17655-show-new-account-instructions-heading-on-purchase-wizard branch February 5, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants