[MGNLUI-5892] I18nisation of search results broke file-based search result suppliers Created: 22/May/20  Updated: 25/May/20  Resolved: 25/May/20

Status: Closed
Project: Magnolia UI
Component/s: None
Affects Version/s: 6.2.1
Fix Version/s: None

Type: Bug Priority: Critical
Reporter: Ilgun Ilgun Assignee: Unassigned
Resolution: Not an issue Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 5m
Original Estimate: Not Specified

Issue Links:
Problem/Incident
causes EXTDAM-118 (Bynder/S3) Find bar through exceptio... Closed
is caused by MGNLUI-5802 Findbar groups should show labels ins... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

To Reproduce:

1)

  • Define a custom search result supplier and name it with a random name 'xyz.yaml'
  • Do a search in the UI and see NPEs at info.magnolia.admincentral.findbar.SearchResultsGrid#getI18nizedSupplierLabel

2)

  • Use external-dam module or e-commerce module
  • Do a search in the UI and see NPEs at info.magnolia.admincentral.findbar.SearchResultsGrid#getI18nizedSupplierLabel

  

 

DEV Hints:

  • The fix should be provided by iterating over the definitions and matching the correct one
  • Trying to fetch with Registry#provider won't work because the id is the file name vs search result has a different type
    • Xyz vs 'product'
  • Write unit tests


 Comments   
Comment by Roman Kovařík [ 23/May/20 ]

Looking at https://git.magnolia-cms.com/projects/ADDON/repos/external-dams/browse/magnolia-external-dam/src/main/java/info/magnolia/external/dam/search/categories/ExternalDamSearchResultSupplier.java#57 it's using name as label. 

If you check info.magnolia.periscope.operation.OperationDefinitionRegistry#newMetadataBuilder, it's using name as id strategy, so the name of the YAML definition and the supplier should match.

Also looking at info.magnolia.external.dam.search.categories.ExternalDamSearchResultSupplierStrategy#construct, it completely ignores the name.

Comment by Aleksandr Pchelintcev [ 25/May/20 ]
  • seems like indeed we have a breach of registry contract there
  • SearchResultSupplierDefinitionRegistry uses name as id resolution strategy (i.e. yaml file name as id)
    ext dam result suppliers, however seem to not follow that contract in implementation and feed hardcoded string “external dam” into the SearchResult
  • I assume that if ExternalDamSearchResultSupplier#getName would return definition#getName the problem would be solved
Generated at Mon Feb 12 09:31:05 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.