site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Assign existing key metric widgets to the new groups metadata

Open zutigrm opened this issue 1 year ago • 9 comments

Feature Description

As part of the key metrics selection panel redesign, all items will be belonging to the respective group. As part of this issue assets/js/components/KeyMetrics/constants.js should be updated to include the list of these groups. key-metrics-widgets should be extended for each metric so it include new metadata object as a property, which will hold group property for now. This group should be extracted in MetricItems and then passed further for filtering where needed.

Relevant figma design can be found here

Check the New Metric Grouping Logic Within Sidebar/Selection Panel - Tabbed Functionality section of the design doc, together with it's sub-sections


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • New groups are introduced to categorise existing metric widgets, in the following way:
    • New groups to be added are: Visitors, Driving traffic, Generating leads, Selling products, Content performance
  • Above mentioned groups should be assigned to their respective key metric widgets, in following categorisation:
    • Visitors
      • Pages per visit
      • Visit length
      • Visits per visitor
      • Top pages by returning visitors
      • New visitors
      • Returning visitors
    • Driving traffic
      • Top traffic source
      • Most engaged traffic source
      • Top converting traffic source
      • Top cities driving traffic
      • Top countries driving traffic
      • Top performing keywords
    • Generating leads
      • Top pages driving leads
      • Top cities driving leads
      • Top traffic source driving leads
    • Selling products
      • Most popular products by pageviews
      • Top cities driving add to cart
      • Top traffic source driving add to cart
      • Top cities driving purchases
      • Top device driving purchases
      • Top traffic source driving purchases
    • Content performance
      • Top recent trending pages
      • Most engaging pages
      • Least engaging pages
      • Most popular authors by pageviews
      • Top categories by pageviews
      • Most popular content by pageviews
      • Top earning content
  • Existing list of metric tiles that are used within selection panel are updated to have new group assign to them
    • These groups should be accessible for filtering, and will be needed in #9378, as that issue relies on the infrastructure added here for further group switching functionality and grouped display of metric items
  • Refer to this section of the design doc and this one

Implementation Brief

  • [x] Update assets/js/components/KeyMetrics/constants.js
    • Add new constant for each group listed in the AC
    • Each constant should hold an object containing SLUG and LABEL, for example for Driving traffic group - {SLUG: 'drivingTraffic', LABEL: 'Driving traffic'}
  • [x] In assets/js/components/KeyMetrics/key-metrics-widgets.js
    • Update the existing key metrics widgets definition to include new property metadata. It should have a group property with correct group constant assigned, for example metadata: { group: DRIVING_TRAFFIC.SLUG }
  • [x] Update metricsListReducer helper function, so that group is properly extracted and retuned in the existing object together with title and description https://github.com/google/site-kit-wp/blob/4ef4ed7e31037a90608b534ec8a47a21ced07d7f/assets/js/components/KeyMetrics/MetricsSelectionPanel/MetricItems.js#L57-L79

Test Coverage

  • No updates needed

QA Brief

  • Console log metricsListReducer(), check each returned audience has a group and the group matches the AC. https://github.com/google/site-kit-wp/blob/d1f773642dc4837eb3e2b39005fb7010d3e16d0b/assets/js/components/KeyMetrics/MetricsSelectionPanel/MetricItems.js#L57-L84
  • Confirm each group is correctly applied in assets/js/components/KeyMetrics/key-metrics-widgets.js matching the AC.

Changelog entry

zutigrm avatar Sep 19 '24 12:09 zutigrm

@zutigrm, could you please update AC to be acceptance criteria without implementation details? Right now there are too many details that affect decisions that IB writer can make which is not something that we should do in AC. Ideally, this AC should explain which groups should be added and which widgets should be in each group. No need to mention which javascript file should be changed.

eugene-manuilov avatar Sep 26 '24 10:09 eugene-manuilov

@eugene-manuilov Thanks, I updated AC. This issue is chained to few other issues in the dependencies, as they are split in few smaller steps, to avoid having one big issue. So in a way steps outlined in design doc need to come together to complete the implementation so I put a bit more details initially. I tried to emphasis on this aspect instead in edited AC.

Let me know what you think.

zutigrm avatar Sep 26 '24 12:09 zutigrm

Thanks, @zutigrm. AC looks better now, but instead of pointing to the document we need to clearly define which groups should be added and which widgets each group should contain because this is our requirement for this task. You can also reference the design doc in AC to let an IB writer where to find explained details of how it should be done.

eugene-manuilov avatar Sep 26 '24 12:09 eugene-manuilov

@eugene-manuilov aha got it, thanks. AC expanded to list the groups and metrics

zutigrm avatar Sep 26 '24 12:09 zutigrm

and Current selection - this is a special group, not assigned to any specific widget

Thanks, @zutigrm. AC almost looks good, the only question whether we really need to define the "current selection" group. This is not actually a group, this is a list of currently selected widgets that will be needed only in the ChipTabGroup component. I think we don't really need to define it in the scope of this task and leave it to be implemented within that component. So let's get rid of it from AC, after that AC will look good.

eugene-manuilov avatar Sep 26 '24 13:09 eugene-manuilov

@eugene-manuilov You are right, AC updated. Thanks

zutigrm avatar Sep 26 '24 13:09 zutigrm

Thanks. AC ✔️

eugene-manuilov avatar Sep 26 '24 13:09 eugene-manuilov

IB :heavy_check_mark:

eugene-manuilov avatar Oct 01 '24 09:10 eugene-manuilov

Marking blocked by #9162 so that we can get a the Top device driving purchases widget in to apply the metadata to.

benbowler avatar Oct 15 '24 09:10 benbowler

QA:Eng

✅ Verified that all KMW have associated groups according to the AC

zutigrm avatar Nov 06 '24 15:11 zutigrm