Unexpected output: Affected paths: no mapping - resubmit patch.
⚙ D397 MINC-197 - Improve user experience for "E-commerce > Sales Report" section
Page MenuHomeIn-Portal Phabricator
Differential D397

MINC-197 - Improve user experience for "E-commerce > Sales Report" section
ClosedPublic
Actions

Authored by alex on Mar 3 2021, 6:16 AM.

Details

Reviewers
erik
Commits
rMINC16696: Fixes MINC-197 - Improve user experience for "E-commerce > Sales Report" section
Test Plan

Preparation (admin console)

  1. login to Admin Console
  2. go to Website & Content → Structure & Data → Products section
  3. create sub category A section in it
  4. create sub category B section in it
  5. create a Product A product in Products > sub category A section
  6. create a Product B product in Products > sub category B section
  7. go to User Management → Users section
  8. create a user-a user
  9. create user-b user

Preparation (front-end)

  1. go to Front-End
  2. login using user-a user
  3. place a sales order with Product A product using Check/MO (Check or Money Order) payment type
  4. logout
  5. login using user-b user
  6. place a sales order with Product B product using Check/MO (Check or Money Order) payment type
  7. logout

Test Plan 1

  1. login to Admin Console
  2. go to E-commerce → Orders section
  3. approve and ship the sales order
  4. go to E-commerce → Sales Report section
  5. try generating each of the reports and confirm, that:
    1. no notices exist on the report configuration page and report result page
    2. data of report can be exported
    3. each report, except Overall has 2 (can be more) rows
    4. hovering over rows in the report (that has several rows) is correctly highlighting both row data and its checkbox cell
    5. after export was made other toolbar buttons work as expected (e.g. opening column picker works)

Test Plan 2

  1. generate Overview report
  2. sort by Marketplace column
  3. return back to report settings page
  4. generate By Product report
  5. confirm, that no SQL error happens

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.2.x
Lint
Lint ErrorsExcuse: Not fixing issues, that weren't introduced in this task
SeverityLocationCodeMessage
Errormodules/in-commerce/units/reports/reports_config.php:91PHPCS.E.CodingStandard.Arrays.Array.SpaceAfterKeywordCodingStandard.Arrays.Array.SpaceAfterKeyword
Errormodules/in-commerce/units/reports/reports_config.php:91PHPCS.E.Generic.PHP.LowerCaseKeyword.FoundGeneric.PHP.LowerCaseKeyword.Found
Errormodules/in-commerce/units/reports/reports_event_handler.php:118PHPCS.E.CodingStandard.Commenting.InlineComment.InvalidEndCharCodingStandard.Commenting.InlineComment.InvalidEndChar
Errormodules/in-commerce/units/reports/reports_event_handler.php:118PHPCS.E.CodingStandard.Commenting.InlineComment.NotCapitalCodingStandard.Commenting.InlineComment.NotCapital
Errormodules/in-commerce/units/reports/reports_event_handler.php:118PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.ContentAfterOpenCodingStandard.WhiteSpace.ControlStructureSpacing.ContentAfterOpen
Errormodules/in-commerce/units/reports/reports_event_handler.php:118PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBrace
Errormodules/in-commerce/units/reports/reports_event_handler.php:118PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBrace
Errormodules/in-commerce/units/reports/reports_event_handler.php:121PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errormodules/in-commerce/units/reports/reports_event_handler.php:121PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errormodules/in-commerce/units/reports/reports_event_handler.php:121PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errormodules/in-commerce/units/reports/reports_event_handler.php:121PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errormodules/in-commerce/units/reports/reports_event_handler.php:166PHPCS.E.CodingStandard.Commenting.InlineComment.InvalidEndCharCodingStandard.Commenting.InlineComment.InvalidEndChar
Errormodules/in-commerce/units/reports/reports_event_handler.php:166PHPCS.E.CodingStandard.Commenting.InlineComment.NotCapitalCodingStandard.Commenting.InlineComment.NotCapital
Errormodules/in-commerce/units/reports/reports_event_handler.php:166PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.ContentAfterOpenCodingStandard.WhiteSpace.ControlStructureSpacing.ContentAfterOpen
Errormodules/in-commerce/units/reports/reports_event_handler.php:166PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBrace
Errormodules/in-commerce/units/reports/reports_event_handler.php:166PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBrace
Errormodules/in-commerce/units/reports/reports_event_handler.php:169PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errormodules/in-commerce/units/reports/reports_event_handler.php:169PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceAfterCodingStandard.Strings.ConcatenationSpacing.NoSpaceAfter
Errormodules/in-commerce/units/reports/reports_event_handler.php:169PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errormodules/in-commerce/units/reports/reports_event_handler.php:169PHPCS.E.CodingStandard.Strings.ConcatenationSpacing.NoSpaceBeforeCodingStandard.Strings.ConcatenationSpacing.NoSpaceBefore
Errormodules/in-commerce/units/reports/reports_event_handler.php:205PHPCS.E.CodingStandard.Commenting.InlineComment.InvalidEndCharCodingStandard.Commenting.InlineComment.InvalidEndChar
Errormodules/in-commerce/units/reports/reports_event_handler.php:205PHPCS.E.CodingStandard.Commenting.InlineComment.NotCapitalCodingStandard.Commenting.InlineComment.NotCapital
Errormodules/in-commerce/units/reports/reports_event_handler.php:205PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.ContentAfterOpenCodingStandard.WhiteSpace.ControlStructureSpacing.ContentAfterOpen
Errormodules/in-commerce/units/reports/reports_event_handler.php:205PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpaceBeforeCloseBrace
Errormodules/in-commerce/units/reports/reports_event_handler.php:205PHPCS.E.CodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBraceCodingStandard.WhiteSpace.ControlStructureSpacing.SpacingAfterOpenBrace
Unit
No Unit Test Coverage
Build Status
Buildable 1008
Build 1008: arc lint + arc unit

Event Timeline

alex created this revision.Mar 3 2021, 6:16 AM
alex requested review of this revision.Mar 3 2021, 6:16 AM
alex edited the test plan for this revision. (Show Details)Mar 3 2021, 7:34 AM
erik accepted this revision.Mar 3 2021, 9:14 AM

Test plan is all right. But, beyond this patch, exists SQL error when doing "View Chart" for "By Product" and "By Section" cases. Problematic SQL is like

SELECT LEFT(c.l1_Name, 60) AS Name, c.CategoryId, SUM() as Metric
   FROM inp_Orders AS o
   LEFT JOIN inp_OrderItems AS od ON od.OrderId = o.OrderId
   LEFT JOIN inp_Products AS p ON p.ProductId = od.ProductId
   LEFT JOIN inp_CategoryItems AS ci ON ci.ItemResourceId = p.ResourceId
   LEFT JOIN inp_Categories AS c ON c.CategoryId = ci.CategoryId
   WHERE o.Status IN (4,6) AND o.OrderDate >= 1614718800 AND o.OrderDate <= 1614805199
   GROUP BY c.CategoryId
   HAVING NOT ISNULL(CategoryId)
   ORDER BY Metric DESC

There is "SUM() as Metric" column, where SUM argument is missing.

This revision is now accepted and ready to land.Mar 3 2021, 9:14 AM
alex added a comment.Mar 3 2021, 1:26 PM
In D397#7804, @erik wrote:

Test plan is all right. But, beyond this patch, exists SQL error when doing "View Chart" for "By Product" and "By Section" cases. Problematic SQL is like

SELECT LEFT(c.l1_Name, 60) AS Name, c.CategoryId, SUM() as Metric
   FROM inp_Orders AS o
   LEFT JOIN inp_OrderItems AS od ON od.OrderId = o.OrderId
   LEFT JOIN inp_Products AS p ON p.ProductId = od.ProductId
   LEFT JOIN inp_CategoryItems AS ci ON ci.ItemResourceId = p.ResourceId
   LEFT JOIN inp_Categories AS c ON c.CategoryId = ci.CategoryId
   WHERE o.Status IN (4,6) AND o.OrderDate >= 1614718800 AND o.OrderDate <= 1614805199
   GROUP BY c.CategoryId
   HAVING NOT ISNULL(CategoryId)
   ORDER BY Metric DESC

There is "SUM() as Metric" column, where SUM argument is missing.

Please report this separately via http://community.in-portal.org/pages/createpage-entervariables.action?templateId=8093698&spaceKey=BUG&title=&newSpaceKey=BUG&fromPageId=983082 page.

alex planned changes to this revision.Mar 4 2021, 5:29 AM
alex updated this revision to Diff 986.Mar 4 2021, 5:32 AM

Removed artifacts resulted after adding numberic IDField to every report grid:

  1. removed ProductId/CategoryId/PortalUserId column from the CSV report export
  2. removed ID option from Metric dropdown in the View Chart popup
This revision is now accepted and ready to land.Mar 4 2021, 5:32 AM
alex requested review of this revision.Mar 4 2021, 5:33 AM
erik accepted this revision.Mar 4 2021, 5:48 AM
This revision is now accepted and ready to land.Mar 4 2021, 5:48 AM
alex updated this revision to Diff 988.Mar 4 2021, 7:20 AM

Fixed outdating sorting resetting code on report change. Before the fix user would have got an SQL error.

alex edited the test plan for this revision. (Show Details)Mar 4 2021, 7:22 AM
This revision was landed with ongoing or failed builds.Jul 8 2022, 3:18 AM
Closed by commit rMINC16696: Fixes MINC-197 - Improve user experience for "E-commerce > Sales Report" section (authored by alex, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

DiffIDBaseDescriptionCreatedLintUnit
BaseBase
Diff 198516671Mar 3 2021, 6:16 AM
★
★
Diff 298616671Removed artifacts resulted after adding numberic IDField to every report grid:Mar 4 2021, 5:32 AM
★
★
Diff 398816671Fixed outdating sorting resetting code on report change. Before the fix user…Mar 4 2021, 7:20 AM
★
★
Diff 4104416695rMINC16696Jul 8 2022, 3:18 AM
★
★

Diff 985

View Options

modules/in-commerce/admin_templates/reports/results.tpl

Loading...
View Options

modules/in-commerce/units/reports/reports_config.php

Loading...
View Options

modules/in-commerce/units/reports/reports_event_handler.php

Loading...
View Options

modules/in-commerce/units/reports/reports_tag_processor.php

Loading...
In-Portal CMS · In-Portal Community Pages · In-Portal Issue Tracker · In-Portal Repository Browser · Quality Assurance · In-Portal CI Server