Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Add RestrictionComments to CustomsInfo Create parameter set #598

Conversation

nwithan8
Copy link
Member

Description

Adds RestrictionComments parameter to the CustomsInfo.Create parameter set, and enforces its presence based on the value of RestrictionType (RestrictionComments must be set if RestrictionType is set (not null) and is anything other than "none")

Closes #597

Adds expanded TopLevelRequestParameterDependentsAttribute and NestedRequestParameterDependentsAttribute to accommodate for mix of value- and presence-based independent-dependent relationships. New possible enforcements, e.g.:

  • Parameter B must (not) be set if Parameter A is (not) a certain value
  • Parameter B must (not) be a certain value if Parameter A is (not) set

Testing

  • Add unit tests to confirm inter-dependency Parameter checks
  • Add unit test to confirm RestrictionComments validation based on RestrictionType
  • Update test fixtures accordingly
  • Re-record cassettes as needed

Pull Request Type

Please select the option(s) that are relevant to this PR.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement (fixing a typo, updating readme, renaming a variable name, etc)

… to existing presence-based checks

- Add unit tests to confirm inter-dependency Parameter checks
- Add `RestrictionComments` parameter to `CustomsInfo` create parameter set
- Mark `RestrictionComments` presence as dependent on `RestrictionType` value
- Add unit test to confirm `RestrictionComments` validation based on `RestrictionType`
- Update test fixtures accordingly
@nwithan8 nwithan8 requested a review from a team as a code owner October 16, 2024 09:47
@nwithan8 nwithan8 linked an issue Oct 16, 2024 that may be closed by this pull request
1 task
@nwithan8 nwithan8 force-pushed the 597-feat-add-restrictioncomments-to-parameterscustomsinfocreatecs branch from f8f17e1 to 90b1979 Compare October 16, 2024 18:44
Copy link
Member

@Justintime50 Justintime50 left a comment

Choose a reason for hiding this comment

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

Huh, ya learn something new every day. I've never seen these customs values used in the wild.

I'm seeing some unrelated cassettes re-recorded here, were they expired? I'd recommend not including unrelated cassette changes if we can avoid it.

@nwithan8
Copy link
Member Author

Huh, ya learn something new every day. I've never seen these customs values used in the wild.

I'm seeing some unrelated cassettes re-recorded here, were they expired? I'd recommend not including unrelated cassette changes if we can avoid it.

This kind of ballooned out, as the CustomsInfo fixture affected Shipment fixtures, which affects a lot of cassettes.

@nwithan8 nwithan8 merged commit ad25818 into master Oct 21, 2024
14 checks passed
@nwithan8 nwithan8 deleted the 597-feat-add-restrictioncomments-to-parameterscustomsinfocreatecs branch October 21, 2024 23:36
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.

[Feat]: Add RestrictionComments to Parameters.CustomsInfo.Create.cs
3 participants