Skip to content

Improve dashboard charts - migrate to chart.js #26923

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 12 commits into from
Mar 24, 2020
Merged

Improve dashboard charts - migrate to chart.js #26923

merged 12 commits into from
Mar 24, 2020

Conversation

Bartlomiejsz
Copy link
Contributor

@Bartlomiejsz Bartlomiejsz commented Feb 19, 2020 •
edited
Loading

Description (*)

This PR aims to migrate dashboard charts to chart.js library instead of static images.
Fixes magento/community-features#186
This is preview of new charts:
image

Related Pull Requests

Manual testing scenarios (*)

  1. Enable Dashboard Charts
  2. Place some orders with different dates and amounts, process them to Complete status
  3. Go to MBO Dashboard
  4. Dynamic charts should be visible

Questions or comments

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)

@Bartlomiejsz Bartlomiejsz added the WIP Work In Progress label Feb 19, 2020
@m2-assistant
Copy link

m2-assistant bot commented Feb 19, 2020

Hi @Bartlomiejsz. 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

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

@Bartlomiejsz Bartlomiejsz mentioned this pull request Feb 19, 2020
Closed
@ihor-sviziev ihor-sviziev self-assigned this Feb 19, 2020
@ihor-sviziev
Copy link
Contributor

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, here is your new Magento instance.
Admin access: https://pr-26923.instances.magento-community.engineering/admin_6356
Login: 63a15b5d Password: a49480ba054f
Instance will be terminated in up to 3 hours.

@ihor-sviziev ihor-sviziev changed the title WIP: https://github.com/magento/community-features/issues/186 - Improve dashboard charts - migrate to chart.js WIP: Improve dashboard charts - migrate to chart.js Feb 19, 2020
@ihor-sviziev
Copy link
Contributor

@Bartlomiejsz looks really promising, however you've reused/removed big parts of code that was used originally, some of these parts marked as "@api", so they can't contain backward incompatible changes.
In places where the most logic was changed probably it's better to create new class and deprecate older ones.

@Bartlomiejsz
Copy link
Contributor Author

Sure @ihor-sviziev, I'll work on moving new functionalities to newly create classes, thank you for review.
Got question, it seems that Static Tests are using different rules for CE edition, and different for B2B - there are only two static issues for CE, but multiple for B2B - should I then fix all of those issues, or that means that B2B module configuration is not updated to latest version and I can ignore that?

@ihor-sviziev
Copy link
Contributor

@Bartlomiejsz
Here is answer for your question :)
https://github.com/magento/magento2/pull/26923/checks?check_run_id=454975377
image

@Bartlomiejsz Bartlomiejsz force-pushed the feature/dashboards_charts branch 2 times, most recently from 42756cf to 3f9c990 Compare February 19, 2020 21:16
@ihor-sviziev
Copy link
Contributor

@magento run all tests

@Bartlomiejsz Bartlomiejsz force-pushed the feature/dashboards_charts branch 4 times, most recently from 5a8b58e to 41fbdb2 Compare February 21, 2020 11:25
@lenaorobei
Copy link
Contributor

@Bartlomiejsz, you are right, I just double checked that.
Please just ignore my comment 🙂 The test that was supposed to test charts is not related to the charts at all.

@Bartlomiejsz
Copy link
Contributor Author

@lenaorobei from what I can see now, it seems it is related, at least was :) It was checking if after placing an order image chart is displayed. So this may indeed require some adjustments after my changes, I'll try to do that, will keep you posted in case of issues

@lenaorobei
Copy link
Contributor

MC-31973 is approved and we are ready to move forward with it.

@ghost ghost assigned lenaorobei Mar 16, 2020
@magento-engcom-team
Copy link
Contributor

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

@Bartlomiejsz
Copy link
Contributor Author

Hi @lenaorobei, in that case changes to this single MFTF test I should probably introduce as separate PR once this will be merged, am I right?

@lenaorobei
Copy link
Contributor

@Bartlomiejsz yes, let's do separate PR. Thanks.

@engcom-Golf
Copy link
Contributor

@magento run all tests

@lenaorobei
Copy link
Contributor

@magento run Integration Tests

magento-engcom-team pushed a commit that referenced this pull request Mar 24, 2020
@lenaorobei
ENGCOM-6988: Improve dashboard charts - migrate to chart.js #26923
78e804f
@magento-engcom-team magento-engcom-team merged commit 4b5e3cd into magento:2.4-develop Mar 24, 2020
@m2-assistant
Copy link

m2-assistant bot commented Mar 24, 2020

Hi @Bartlomiejsz, 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.

@ihor-sviziev
Copy link
Contributor

Yeah! It was finally merged! That’s amazing contribution @Bartlomiejsz! Thank you so much!

@Bartlomiejsz
Copy link
Contributor Author

Thanks @ihor-sviziev and thank you for your help on this!

@Bartlomiejsz Bartlomiejsz deleted the feature/dashboards_charts branch September 19, 2020 09:57
@sdzhepa sdzhepa mentioned this pull request May 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihor-sviziev ihor-sviziev ihor-sviziev approved these changes

@VladimirZaets VladimirZaets VladimirZaets approved these changes

@edenduong edenduong edenduong approved these changes

@lenaorobei lenaorobei lenaorobei approved these changes

@naydav naydav naydav left review comments

@melnikovi melnikovi melnikovi left review comments

Assignees

@naydav naydav

@ihor-sviziev ihor-sviziev

@VladimirZaets VladimirZaets

@edenduong edenduong

@melnikovi melnikovi

@lenaorobei lenaorobei

@engcom-Alfa engcom-Alfa

@engcom-Golf engcom-Golf

Labels
Area: Design/Frontend Area: Frontend Area: Lib/Frontend Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: advanced Award: category of expertise Award: special achievement Award: test coverage Component: Backend Partner: Fast White Cat partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve dashboard charts