Issue 24 #28
Merged
Issue 24 #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- add state fix for console command - add command option resolving fix - add shipments aggregator - add shipments count aggregator unit tests coverage.
- add categories count aggregator based on category status, category menu status and store code - add unit tests coverage for categories count aggregator.
- adjust 'magento_store_count_total' metric; - add status label to stores count; - adjust unit tests coverage;
- Add magento_website_count_total metric. - add unit tests coverage.
- Update readme.md file.
- Fix code styles.
- Fix code styles.
|
I believe it does not make sense to fix alignments, as difference in PHP Storm settings. |
| { | ||
| private const METRIC_CODE = 'magento_catalog_category_count_total'; | ||
| private const T_ATT = 'm2_eav_attribute'; | ||
| private const T_CAT_ENT_INT = 'm2_catalog_category_entity_int'; |
There was a problem hiding this comment.
What are these abbreviations? Test? Table? I'd suggest to be more explicit here.
|
Hi @swnsma, thank you for the PR. I am gonna look at the CI stuff next week. I guess it is time to check out the Github Actions :D. Besides that, your code looks fine. I don't actually like these crazy big SQL's but I guess we have to use them in order to be as performant as possible. Will try the metrics and give feedback next as well |
Fix code review notices.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related ticket:
#24
Metrics added:
Metrics changed:
Adjusted unit tests coverage.
Updated README.md
Points to discuss:
Possible suggestions:
menu_statuslabel and collect only category statusmenu_statusif category is disabled (= leave only 3 metrics, if category disabled - consider that it won't be showed in the menu).It will require separated SQL query to fetch data for admin store (e.g. all available categories). Should we collect this data too, or only leave data collection per actual store?