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

Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url #30410

Merged
merged 24 commits into from
Dec 23, 2020

Conversation

GovindaSharma
Copy link
Contributor

@GovindaSharma GovindaSharma commented Oct 10, 2020

Preconditions (*)

Magento 2.4 Develop

Description (*)

Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url

Fixes #30424

Related Pull Requests

https://github.com/magento/partners-magento2ee/pull/417

Manual testing scenarios (*)

1.Create An Order In Magento

2.Create invoice, shipment and creditmemo of that Order

3.View the created shipment,invoice and creditmemo

4.Now change the shipment id ,invoiceid and creditmemo id from url(use that id which does not exist) and run the url.

Expected result (*)

A proper error message should come

Actual Result (*)

error page occurs

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Resolved issues:

  1. resolves [Issue] Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url #30424: Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url

@m2-assistant
Copy link

m2-assistant bot commented Oct 10, 2020

Hi @GovindaSharma. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE,
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

@GovindaSharma GovindaSharma changed the title Dcmm2020 Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url Oct 10, 2020
@GovindaSharma
Copy link
Contributor Author

#dmcdindia2020

@sivaschenko
Copy link
Member

@magento create issue

@sivaschenko sivaschenko added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Oct 10, 2020
@gabrieldagama
Copy link
Contributor

@magento run all tests

@GovindaSharma
Copy link
Contributor Author

@magento give me test instance

@magento-deployment-service
Copy link

Hi @GovindaSharma. Thank you for your request. I'm working on Magento instance for you.

@magento-deployment-service
Copy link

@rogyar rogyar self-assigned this Oct 10, 2020
Copy link
Contributor

@rogyar rogyar left a comment

Choose a reason for hiding this comment

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

Hi @GovindaSharma. Thank you for your contribution. Great Job!
Please, check my comments below. Please note, my recommendations are also applicable for all three entities (I've left the comments for credit memo only)

<amOnPage url="{{AdminCreditmemoViewPage.url}}/{{identifier}}" stepKey="amOnCreditmemoViewPage"/>
<waitForPageLoad stepKey="waitForPageLoad"/>
</actionGroup>
</actionGroups>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, add a new line at the end of the file

<see selector="{{AdminMessagesSection.error}}" userInput='This creditmemo no longer exists.'
stepKey="seeErrorMessage"/>
</test>
</tests>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, add a new line at the end of the file


<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
<actionGroup name="AdminGoToCreditmemoViewActionGroup">
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest renaming this action group to something like AdminOpenCreditMemoByEntityIdActionGroup. Just to use more or less the same naming as, for example, in the following implementation

<actionGroup name="AdminOpenOrderByEntityIdActionGroup">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rogyar .Thaks for your review. But can you please let me know what is the need of renaming the action group name? i have used this name based on other action group name.

Copy link
Contributor

Choose a reason for hiding this comment

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

This one is not critical, but you may find the example of another action group in the Sales module in my previous comment. My recommendation is more about keeping the names transparent, i.e.

  • AdminOpenCreditMemoByEntityIdActionGroup (new)
  • AdminOpenOrderByEntityIdActionGroup (existing)


<seeInCurrentUrl url="{{AdminCreditmemosGridPage.url}}" stepKey="redirectToCreditmemosGridPage"/>

<see selector="{{AdminMessagesSection.error}}" userInput='This creditmemo no longer exists.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use the following action group instead

<actionGroup name="AssertMessageInAdminPanelActionGroup">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rogyar what is the need of changing action group name for this file as it belongs to Magento_Backend module, i have done commits on Sales and Shipping module.

Copy link
Contributor

Choose a reason for hiding this comment

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

The main reason is following the best practices and building the test body using action groups only. If we already have an action group for some purpose it's recommended to use it rather than introduce a new check.


<waitForPageLoad stepKey="waitForPageLoad"/>

<seeInCurrentUrl url="{{AdminCreditmemosGridPage.url}}" stepKey="redirectToCreditmemosGridPage"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest using the following action group instead if possible

<actionGroup name="AdminAssertPageTitleActionGroup">

@engcom-Alfa engcom-Alfa added QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope and removed QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) labels Dec 1, 2020
@engcom-Charlie
Copy link
Contributor

@magento run all tests

magento-engcom-team pushed a commit that referenced this pull request Dec 23, 2020
@magento-engcom-team magento-engcom-team merged commit d7f15e9 into magento:2.4-develop Dec 23, 2020
@m2-assistant
Copy link

m2-assistant bot commented Dec 23, 2020

Hi @GovindaSharma, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@GovindaSharma
Copy link
Contributor Author

@magento give me 2.4-develop instance

@magento-deployment-service
Copy link

Hi @GovindaSharma. Thank you for your request. I'm working on Magento instance for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: complex Award: test coverage Component: Sales Component: Shipping Event: dmcdindia2020 Partner: Cedcommerce partners-contribution Pull Request is created by Magento Partner Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Progress: accept QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope Release Line: 2.4 Severity: S0 A problem that is blocking the ability to work. An immediate fix is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue] Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url
7 participants