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

#27 [Inspection] Observer declaration check for duplicates #59

Merged

Conversation

vasilii-b
Copy link

@vasilii-b vasilii-b commented Mar 25, 2020

Description

This PR adds a new code inspection to avoid accidentally overriding an observer.

image

This PR solves next issues

  1. Inspection for possible observer name duplication  #27 - Inspection for possible observer name duplication

More details

ℹ️ The Observer Inspections adds warnings with the following information

1. Observer name already used in the same file
same-file-duplicate-found

2. Observer name already used in the scope of other modules
(see screenshot from #3)

3. Information about the duplicated name scope: Vendor_Module/etc, Vendor_Module/etc/adminhtml or Vendor_Module/etc/frontend

diplicated-name-hints-with-scope

4. 🙅‍♂ No warning for the disabled observer
disabled-observer-no-warning

5. 📖Details of why such situation occurs and how to handle it
inspection-description

@vasilii-b vasilii-b changed the title [Inspection] Observer declaration check for duplicated [Inspection] Observer declaration check for duplicates Mar 25, 2020
@vasilii-b vasilii-b changed the title [Inspection] Observer declaration check for duplicates #27 [Inspection] Observer declaration check for duplicates Mar 25, 2020
return new XmlElementVisitor() {
private final String moduleXmlFileName = ModuleXml.getInstance().getFileName();
private static final String eventsXmlFileName = "events.xml";//todo: move to separate file as done to ModuleXml
private static final String duplicatedObserverNameSameFileProblemDescription = "Observer name already used in this file. For more details see Inspection Description.";
Copy link
Contributor

@VitaliyBoyko VitaliyBoyko Mar 25, 2020

Choose a reason for hiding this comment

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

The observer name already used in this file. For more details see Inspection Description.

private static final String eventsXmlFileName = "events.xml";//todo: move to separate file as done to ModuleXml
private static final String duplicatedObserverNameSameFileProblemDescription = "Observer name already used in this file. For more details see Inspection Description.";
private static final String duplicatedObserverNameProblemDescription =
"Observer name \"%s\" for event \"%s\" is already used in the module \"%s\" (%s scope). For more details see Inspection Description.";
Copy link
Contributor

@VitaliyBoyko VitaliyBoyko Mar 25, 2020

Choose a reason for hiding this comment

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

The observer name...

@VitaliyBoyko
Copy link
Contributor

One more thing. Don't forget to update CHANGELOG.md with the new functionality.

@vasilii-b
Copy link
Author

Hi @VitaliyBoyko,
The requested changes have been added ✅
Please let me know should there be needed more adjustments.
Thanks!

@vasilii-b
Copy link
Author

Hi @VitaliyBoyko,
My bad here. I didn't get your message initially.
All possible areas are added now. Example:
all-possible-area-scopes-added
Please let me know if I missed smth this time.

@VitaliyBoyko VitaliyBoyko merged commit 6a217b1 into magento:1.0.0-develop Mar 27, 2020
@vasilii-b vasilii-b deleted the observer-declaration-check branch March 27, 2020 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants