WidgetsList->remove uses translated $widgetName => cannot actually remove widgets #19389

Prinzhorn · 2022-06-22T16:46:40Z

I'm trying to use Widget.filterWidgets to remove certain widgets we don't need. This works for removing a whole category, e.g. $list->remove('Marketplace_Marketplace'); removes the entire menu with all submenus. However, you cannot reliably use the second argument. I was trying to remove a particular menu, e.g. "Behaviour" -> "Site Search". So I tried several things such as $list->remove('General_Actions', 'Actions_SubmenuSitesearch'); but I didn't seem to get anywhere. So I tried to track this down and added:

file_put_contents('/tmp/test', $widgetName.' >>> '.($widget->getName())."\n", FILE_APPEND);

right here between the two ifs:

matomo/core/Widget/WidgetsList.php

Lines 142 to 143 in 260f512

    
           if ($widget->getCategoryId() === $widgetCategoryId) { 
        
               if (!$widgetName || $widget->getName() === $widgetName) {

And to my surprise this is what I get for $list->remove('General_Actions', 'Actions_SubmenuSitesearch');. A mix of keys like Actions_WidgetEntryPageTitles and actual translations like Pages Following a Site Search.

Actions_SubmenuSitesearch >>>
Actions_SubmenuSitesearch >>>
Actions_SubmenuSitesearch >>> Transitions_Transitions
Actions_SubmenuSitesearch >>> General_Pages
Actions_SubmenuSitesearch >>> Entry pages
Actions_SubmenuSitesearch >>> Actions_WidgetEntryPageTitles
Actions_SubmenuSitesearch >>> Exit pages
Actions_SubmenuSitesearch >>> Exit page titles
Actions_SubmenuSitesearch >>> Page titles
Actions_SubmenuSitesearch >>> Site Search Keywords
Actions_SubmenuSitesearch >>> Pages Following a Site Search
Actions_SubmenuSitesearch >>> Search Keywords with No Results
Actions_SubmenuSitesearch >>> Page Titles Following a Site Search
Actions_SubmenuSitesearch >>> Search Categories
Actions_SubmenuSitesearch >>> Outlinks
Actions_SubmenuSitesearch >>> Downloads
Actions_SubmenuSitesearch >>> VisitorInterest_VisitsPerDuration
Actions_SubmenuSitesearch >>> VisitorInterest_VisitsPerNbOfPages
Actions_SubmenuSitesearch >>> Visits by Visit Number
Actions_SubmenuSitesearch >>> VisitorInterest_WidgetVisitsByDaysSinceLast
Actions_SubmenuSitesearch >>> VisitFrequency_WidgetGraphReturning
Actions_SubmenuSitesearch >>> VisitFrequency_WidgetOverview
Actions_SubmenuSitesearch >>> PagePerformance_EvolutionOverPeriod
Actions_SubmenuSitesearch >>>
Actions_SubmenuSitesearch >>> Actions_PageUrls
Actions_SubmenuSitesearch >>> Actions_SubmenuPageTitles

If I switch my user to German I get

Actions_SubmenuSitesearch >>>
Actions_SubmenuSitesearch >>>
Actions_SubmenuSitesearch >>> Transitions_Transitions
Actions_SubmenuSitesearch >>> General_Pages
Actions_SubmenuSitesearch >>> Einstiegsseiten
Actions_SubmenuSitesearch >>> Actions_WidgetEntryPageTitles
Actions_SubmenuSitesearch >>> Ausstiegsseiten
Actions_SubmenuSitesearch >>> Titel der Ausstiegsseite
Actions_SubmenuSitesearch >>> Seitentitel
Actions_SubmenuSitesearch >>> Suchbegriffe (interne Suche)
Actions_SubmenuSitesearch >>> Besuchte Seiten nach einer internen Suche
Actions_SubmenuSitesearch >>> Suchbegriffe ohne Ergebnisse
Actions_SubmenuSitesearch >>> Seitentitel nach einer internen Suche
Actions_SubmenuSitesearch >>> Suchkategorien
Actions_SubmenuSitesearch >>> Ausgehende Verweise
Actions_SubmenuSitesearch >>> Downloads
Actions_SubmenuSitesearch >>> VisitorInterest_VisitsPerDuration
Actions_SubmenuSitesearch >>> VisitorInterest_VisitsPerNbOfPages
Actions_SubmenuSitesearch >>> Besuche nach Besuchsanzahl
Actions_SubmenuSitesearch >>> VisitorInterest_WidgetVisitsByDaysSinceLast
Actions_SubmenuSitesearch >>> VisitFrequency_WidgetGraphReturning
Actions_SubmenuSitesearch >>> VisitFrequency_WidgetOverview
Actions_SubmenuSitesearch >>> PagePerformance_EvolutionOverPeriod
Actions_SubmenuSitesearch >>>
Actions_SubmenuSitesearch >>> Actions_PageUrls
Actions_SubmenuSitesearch >>> Actions_SubmenuPageTitles

So clearly there is something wrong. Comparing the passed $widgetName with a translation and not with a consistent key has no place in an API. I also still haven't been able to actually remove anything but an entire menu tree. Surely I must be doing something wrong?

Expected Behavior

I can use WidgetsList->remove to remove a specific widget.

Current Behavior

I can't and for most widgets it would break once the user uses a different language.

I personally thought I can just use the subcategory URL parameter to figure out what the second argument needs to be if I want to remove a particular menu item. But that doesn't seem to be it, because subcategory are what actually contain the widgets? So I think we actually have two different issues here:

The API is broken (I think we can agree that it shouldn't log German translations in the example above)
The document should be improved. It says

The name of the widget to remove eg 'VisitTime_ByServerTimeWidgetName'. If not supplied, all widgets within that category will be removed.

But how do I get VisitTime_ByServerTimeWidgetName? If I look at the UI how would I ever get the correct translation key? In fact it only ever appears in the documentation (https://github.com/matomo-org/matomo/search?q=VisitTime_ByServerTimeWidgetName) and doesn't seem to actually be an existing key? So I can't even use it to find relevant code to find a way to figure out other keys.

Possible Solution

Use the translation key and not the translation? And also add info to the documentation how to find the key.

Steps to Reproduce (for Bugs)

I hope my explanation above is enough to understand the problem

Your Environment

Docker Matomo 4.10.1

version: '3'

services:
  db:
    image: mariadb
    command: --max-allowed-packet=64MB
    restart: always
    volumes:
      - db:/var/lib/mysql
    environment:
      - MYSQL_ROOT_PASSWORD=password
    env_file:
      - ./db.env

  app:
    image: matomo
    restart: always
    volumes:
      #     - ./config:/var/www/html/config
      #     - ./logs:/var/www/html/logs
      - matomo:/var/www/html
    environment:
      - MATOMO_DATABASE_HOST=db
    env_file:
      - ./db.env
    ports:
      - 8080:80

volumes:
  db:
  matomo:

The text was updated successfully, but these errors were encountered:

bx80 · 2022-06-22T23:03:41Z

Thanks for reporting this @Prinzhorn, the detail is much appreciated.

It looks like there is an inconsistency in naming with some widgets setting their name to the translated text and some using the translation key. For API use there should be a consistent language independent name, like the translation key.

Translation keys for widgets could be found by looking in plugins/[plugin name]/lang/en.json but that would require some guesswork and checking multiple places. Perhaps the widget translation key could be shown discretely somewhere on the widget UI via a hover over, or an API call added to provide a list.

I'll mark this issue for prioritization by our product team.

Prinzhorn · 2022-06-23T16:55:17Z

Perhaps the widget translation key could be shown discretely somewhere on the widget UI via a hover over, or an API call added to provide a list.

What I've seen in some frameworks and what might work well is outputting HTML comments when in development mode. So that you get something like this:

<!--<General_Pages>-->
...The actual widget HTML...
<!--</General_Pages>-->

So that when inspecting the HTML I can see what each widget is called (e.g. General_Pages) right in the HTML. Other information that is useful could also be added in development mode this way. For me personally it's the most natural way to just click "Inspect" on the widget in question.

I don't know anything about the Matomo architecture but this might be trivial to add in some central place that renders widgets (server and client side).

Could you clarify one thing for me: in order to remove a submenu (an entire subcategory url parameter) I need to remove all widget inside of it and it will disappear? Given the following screenshot:

I can easily remove the entire "Visitors" menu by using $list->remove('General_Visitors'). To remove the "Overview" submenu I would need to remove both the widgets "Visits Over Time" and "Visits Overview"? Is that correct? It appears there's a gap in the API to remove an entire subcategory, in this case remove General_Overview from General_Visitors. Because that's what I'm trying to do.

sgiehl · 2022-06-24T08:13:44Z

@Prinzhorn Do I understand that correctly, that you actually want to remove the menu entry? Or do you also want to make the widgets unavailable on the dashboard?

Prinzhorn · 2022-06-24T08:37:19Z

@sgiehl we are setting up Matomo for some clients using only log importing. So I was asked to remove some menus because they might confuse the users and cause questions (e.g. anything "Real-time"). We have configured dashboards with only relevant widgets but yes, it totally makes sense when useless (with log importing) widgets would be entirely removed. So if the current API was fixed I guess that solves all my problems, but being able to remove entire subcategories would also make things easier.

sgiehl · 2022-06-24T10:10:20Z

@Prinzhorn The "problem" with the widgetlist here is, that it is generated from two sources. Once source is the manually defined widgets, that are defined using a WidgetConfig. For those the name is untranslated. The other source are the Report classes, which might configure custom widgets or use the default factory. The default factory uses the reports name, which is currently translated.
It might possibly be a better approach for you the use the event Report.filterReports and remove the report classes you don't want to use. This will automatically also remove the widgets for those reports and make the report unavailable through API.
Nevertheless you should be careful which reports you remove, as some report might rely on other reports.

sgiehl · 2022-06-24T11:59:12Z

Note: I think it might make more sense to introduce a new method WidgetList::removeByUniqueId instead of adjusting all reports to have their names untranslated by default. The unique id is generated based on the widgets module and action and optional parameters (like a goal id).

Copying this from the docs caused > Unmatched '}' and fixing it caused > Call to undefined method Piwik\Plugins\API\Reports\Get::getCategory() and logging `getCategoryId()` shows that `Actions` is not correct either. But using `General_Actions` doesn't remove any menus either, even though `unset` is called. I feel like the first person using these APIs (refs matomo-org#19389)

Prinzhorn · 2022-06-27T08:34:20Z

It might possibly be a better approach for you the use the event Report.filterReports and remove the report classes you don't want to use. This will automatically also remove the widgets for those reports and make the report unavailable through API.
Nevertheless you should be careful which reports you remove, as some report might rely on other reports.

@sgiehl I tried that, see #19414 even with the fixed docs I can't seem to get it working. Is anyone using these APIs? grep.app doesn't show any results other than this repo (https://grep.app/search?q=Report.filterReports).

Note: I think it might make more sense to introduce a new method WidgetList::removeByUniqueId instead of adjusting all reports to have their names untranslated by default. The unique id is generated based on the widgets module and action and optional parameters (like a goal id).

Makes sense, otherwise this would also be a breaking change for people who rely on the current behavior and use English interface exclusively.

Copying this from the docs caused > Unmatched '}' and fixing it caused > Call to undefined method Piwik\Plugins\API\Reports\Get::getCategory() and logging `getCategoryId()` shows that `Actions` is not correct either. But using `General_Actions` doesn't remove any menus either, even though `unset` is called. I feel like the first person using these APIs (refs #19389)

Prinzhorn · 2022-07-25T12:37:53Z

Originally this was confirmed as a bug in filterWidgets. In a comment you recommended filterReports would be better suited for me to remove the menus. However, that doesn't work either as mentioned in this comment #19414 (comment)

I just want to make sure that: Both are fixed and filterReports is confirmed buggy as well.

From what I can tell there is currently no reliable way for me to remove individual submenus in this menu on the left

Is this correct or is there a way?

Prinzhorn added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Jun 22, 2022

bx80 added Bug For errors / faults / flaws / inconsistencies etc. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Jun 22, 2022

bx80 added this to the For Prioritization milestone Jun 22, 2022

Prinzhorn mentioned this issue Jun 27, 2022

Update ReportsProvider.php #19414

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WidgetsList->remove uses translated $widgetName => cannot actually remove widgets #19389

WidgetsList->remove uses translated $widgetName => cannot actually remove widgets #19389

Prinzhorn commented Jun 22, 2022 •

edited

bx80 commented Jun 22, 2022

Prinzhorn commented Jun 23, 2022

sgiehl commented Jun 24, 2022

Prinzhorn commented Jun 24, 2022

sgiehl commented Jun 24, 2022

sgiehl commented Jun 24, 2022

Prinzhorn commented Jun 27, 2022

Prinzhorn commented Jul 25, 2022

Navigation Menu

WidgetsList->remove uses translated $widgetName => cannot actually remove widgets #19389

WidgetsList->remove uses translated $widgetName => cannot actually remove widgets #19389

Comments

Prinzhorn commented Jun 22, 2022 • edited

Expected Behavior

Current Behavior

Possible Solution

Steps to Reproduce (for Bugs)

Your Environment

bx80 commented Jun 22, 2022

Prinzhorn commented Jun 23, 2022

sgiehl commented Jun 24, 2022

Prinzhorn commented Jun 24, 2022

sgiehl commented Jun 24, 2022

sgiehl commented Jun 24, 2022

Prinzhorn commented Jun 27, 2022

Prinzhorn commented Jul 25, 2022

Prinzhorn commented Jun 22, 2022 •

edited