Skip to content

Resolve typo despatch event #18372

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 31, 2018
Merged

Resolve typo despatch event #18372

merged 4 commits into from
Oct 31, 2018

Conversation

neeta-wagento
Copy link
Contributor

Description

Fixed Issues (if relevant)

  1. Typo in dispatched event name #18355: Typo in dispatched event name

Manual testing scenarios

  1. method Magento\CatalogSearch\Model\Indexer\Fulltext\Action\DataProvider::getSearchableAttributes dispatches event with typo in its name
  2. catelogsearch_searchable_attributes_load_after
    instead of
    catalogsearch_searchable_attributes_load_after

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 on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 4, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team magento-engcom-team added Partner: Wagento Pull Request is created by partner Wagento partners-contribution Pull Request is created by Magento Partner Component: AdminNotification Release Line: 2.3 labels Oct 4, 2018
@neeta-wagento neeta-wagento changed the base branch from 2.3-develop to 2.2-develop October 4, 2018 07:05
@orlangur orlangur self-assigned this Oct 4, 2018
@orlangur
Copy link
Contributor

orlangur commented Oct 4, 2018

Unfortunately such change is not allowed: https://devdocs.magento.com/guides/v2.0/contributor-guide/backward-compatible-development/#magento-apis

Checking in Slack whether it's possible to introduce a new event deprecating this one.

@neeta-wagento
Copy link
Contributor Author

@orlangur Hello, Is there any update on this?

@davidalger davidalger removed their request for review October 5, 2018 13:41
orlangur
orlangur requested changes Oct 17, 2018
View reviewed changes
app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Action/DataProvider.php
@@ -317,7 +317,7 @@ public function getSearchableAttributes($backendType = null)
$attributes = $productAttributes->getItems();

$this->eventManager->dispatch(
'catelogsearch_searchable_attributes_load_after',
'catalogsearch_searchable_attributes_load_after',
Copy link
Contributor

Choose a reason for hiding this comment

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

As event cannot be removed, add a comment near old one that it is deprecated and add one more event dispatching, with corrected name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orlangur Thankyou for review. I have added add a comment near old one that it is deprecated and add one more event dispatching, with corrected name.

Copy link
Contributor

Choose a reason for hiding this comment

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

After the change, old event must still be dispatched.

Also, do not specify version near deprecation tag, it is done automatically when needed.

orlangur
orlangur requested changes Oct 27, 2018
View reviewed changes
app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Action/DataProvider.php
@@ -317,7 +317,7 @@ public function getSearchableAttributes($backendType = null)
$attributes = $productAttributes->getItems();

$this->eventManager->dispatch(
'catelogsearch_searchable_attributes_load_after',
'catalogsearch_searchable_attributes_load_after',
Copy link
Contributor

Choose a reason for hiding this comment

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

After the change, old event must still be dispatched.

Also, do not specify version near deprecation tag, it is done automatically when needed.

@magento-engcom-team magento-engcom-team added this to the Release: 2.2.8 milestone Oct 29, 2018
@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-3327 has been created to process this Pull Request

@magento-engcom-team magento-engcom-team mentioned this pull request Oct 31, 2018
Closed
@magento-engcom-team magento-engcom-team merged commit 2647a89 into magento:2.2-develop Oct 31, 2018
magento-engcom-team pushed a commit that referenced this pull request Oct 31, 2018
ENGCOM-3327: Resolve typo despatch event #18372
b2582e7
@magento-engcom-team
Copy link
Contributor

Hi @neeta-wagento. Thank you for your contribution.
We will aim to release these changes as part of 2.2.8.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orlangur orlangur orlangur approved these changes

@okorshenko okorshenko Awaiting requested review from okorshenko

@ishakhsuvarov ishakhsuvarov Awaiting requested review from ishakhsuvarov

@dmanners dmanners Awaiting requested review from dmanners

Assignees

@orlangur orlangur

Labels
Cleanup Component: CatalogSearch Partner: Wagento Pull Request is created by partner Wagento partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.2 Release Line: 2.3
Projects
None yet
Milestone
Release: 2.2.8
Development

Successfully merging this pull request may close these issues.
4 participants
@neeta-wagento @magento-cicd2 @orlangur @magento-engcom-team