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

DT-716 first set changes #23

Merged
merged 8 commits into from
Dec 8, 2023
Merged

DT-716 first set changes #23

merged 8 commits into from
Dec 8, 2023

Conversation

preetamnpr
Copy link
Collaborator

No description provided.

Comment on lines 118 to 122
CarrierScenarioParameters carrierScenarioParameters =
new CarrierScenarioParameters(
"Carrier Service %d".formatted(RANDOM.nextInt(999999)),
generateSchemaValidVesselIMONumber());
generateSchemaValidVesselIMONumber(),
"service name",null,null,null,null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would assume there nulls should have real values in them. If not, the framework would have provided null where a real value should have been and I would expect a schema validation error then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function was never called.. I tried and tested it. The checks were fine.. I m bit confused.. Hence I put the null values.. I will provide the real values in them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gj0dcsa Why do we have this function if it is never called?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That method is the carrier implementation of the SupplyCSP action, and if you put a breakpoint on it you can see that it gets called if you run the all-in-one booking scenarios in automated mode.

If you run in manual mode, testing a carrier, because isInputRequired() returns true, instead of calling that method on a synthetic carrier, the orchestrator relies on the web UI to ask the user to enter them manually.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks George for the info..

generateSchemaValidVesselIMONumber());
generateSchemaValidVesselIMONumber(),
"service name",
"%07d".formatted(RANDOM.nextInt(999999)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

No, a random number is not valid as a HS Code. Please use a proper HS Code and the Commodity Type should match the HS Code (If the HS Code implies it is a leather goods, then Commodity Type should mention some form of leather based good).

Comment on lines 79 to 38
"name": "Asseco DK",
"street": "Kronprinsessegade",
"name": "TODO",
"street": "TODO",
"streetNumber": "54",
"floor": "5. sal",
"postCode": "1306",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please update the address (there is a TODO left here)

Comment on lines 79 to 38
"name": "Asseco DK",
"street": "Kronprinsessegade",
"name": "Mustermann",
"street": "Beispielstreet",
"streetNumber": "54",
"floor": "5. sal",
"postCode": "1306",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was expecting this to be rewritten to DCSA's office or something like that. Currently, it is a mix of a Danish and whatever Beispielstreet classified as (mix of German for "example" and English for "street"?).

Copy link
Collaborator

@nt-gt nt-gt Dec 7, 2023

Choose a reason for hiding this comment

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

For the record, this should be a real address. Though you might be able to prune out some of the fields that are not relevant (check the schema for mandatory fields)

Copy link
Collaborator Author

@preetamnpr preetamnpr Dec 7, 2023

Choose a reason for hiding this comment

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

We used to give always a dummy address. Hence I came up with that name.

@preetamnpr
Copy link
Collaborator Author

Since these changes reviewed and hence merging these changes.

@preetamnpr preetamnpr merged commit 41f00fd into dev Dec 8, 2023
1 check passed
@jkosternl jkosternl deleted the DT-716 branch August 20, 2024 07:18
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.

3 participants