-
Notifications
You must be signed in to change notification settings - Fork 0
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
OBPIH-6882: Fix for Export all items empty db case #30
Conversation
await createInboundPage.addItemsStep.isLoaded(); | ||
await createInboundPage.wizzardSteps.assertActiveStep('Add items'); | ||
|
||
INBOUND2_ID = createInboundPage.getId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need to declare the INBOUND2_ID
as a let
? I don't see you are overriding this value, but maybe I am missing something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise it'd need to be initialized in a different place, so I guess it is fine. Plus I see this is done like that in other places as well.
const ifInboundListPageIsEmpty = | ||
await inboundListPage.table.emptyInboundList.isVisible(); | ||
const ifInboundListPageIsNotEmpty = | ||
await inboundListPage.table.emptyInboundList.isHidden(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need both ifInboundListPageIsEmpty
and ifInboundListPageIsNotEmpty
?
For me, it looks like:
ifInboundListPageIsEmpty
== !ifInboundListPageIsNotEmpty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beside the one that I mentioned there are more code duplications. Is it possible to improve the code to get rid of the other duplications as well?
} | ||
|
||
if (ifInboundListPageIsNotEmpty) { | ||
await test.step('Go to create stock movement', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything in this if is a duplication of the part from lines 98-145. Can you just move this part out of the if above and remove this one? Or is there any kind of difference there?
await createInboundPage.addItemsStep.isLoaded(); | ||
await createInboundPage.wizzardSteps.assertActiveStep('Add items'); | ||
|
||
INBOUND2_ID = createInboundPage.getId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise it'd need to be initialized in a different place, so I guess it is fine. Plus I see this is done like that in other places as well.
Added fix for issue described in https://pihemr.atlassian.net/browse/OBPIH-6882?focusedCommentId=157847