[SIMPROVE-3] SiteDefinitionDomainMapper does not handle all site definition properties Created: 23/Dec/19 Updated: 03/Dec/20 Resolved: 05/Feb/20 |
|
| Status: | Closed |
| Project: | Siteimprove |
| Component/s: | None |
| Affects Version/s: | 1.0.4, 1.0.3, 1.1 |
| Fix Version/s: | 1.1.5 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Riste Drangovski | Assignee: | Adrien Manzoni |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 1d 1h | ||
| Original Estimate: | Not Specified | ||
| Environment: |
issue is not environment specific |
||
| Attachments: |
|
| 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: | |
| Sprint: | Sprint 2 |
| Description |
|
In our project we are using magnolia multisite feature. Magnolia multisite definition supports definition of URIPrefix and handlePrefix. With the multisite configuration magnolia page path is not the same as the page url. Url of this page can be: In case we have configured "/index" as handlePrefix in site configuration. As you can see from siteimprove source code in info.magnolia.connector.siteimprove.mapper.SiteDefinitionDomainMapper ( check attachement) nowhere in siteimprove code you are not handling handlePrefix and URIPrefix site configurations. So when siteimprove widget submits ajax request url of the page is wrong (you are submitting page path not the url of the page that was crawled by siteimprove).
Second issue is that we are using magnolia url translation module: https://wiki.magnolia-cms.com/display/SERVICES/URL+Translation With this module magnolia supports different (translated) urls for same page. For example: if we have page with path: /home/test/page http://www.tcs-mymed.ch/Zuhause/Prüfung/Seite.html based on user language. And again when siteimprove widget sends ajax request, url of the current page is not generated to support url-translation module, you are not using Magnolia's info.magnolia.link.LinkTransformerManager to get the url, so the url is wrong. I don't know if my brief summary is understandable, but I'm available for any questions. Regards, |
| Comments |
| Comment by Adrien Manzoni [ 13/Jan/20 ] |
|
Hi Riste, I am going to implement a new mapper (i ll try as well using the linkmanager). It should solve the case 1.
It might also solve the case 2. However if it doesn't, I will not create a dependency with the url translation module. Then you will probably have to extend that mapper on your side.
I'll try to do it for this week. I'll keep you posted.
|
| Comment by Riste Drangovski [ 14/Jan/20 ] |
|
Hi Adrien, Thanks for your feedback. Please let me know if you need anything from my side. One point regarding siteimprove: Usually magnolia public and author instances have different urls (domains/subdomains/context paths). Maybe it would be nice to have some configuration in the module where we can specify "url prefix" so that url that is sent to siteimprove from author instance is same as scanned url on public instance. Or if you have some other idea it would be great.
Regards,
|
| Comment by Nadine Franzen [ 21/Jan/20 ] |
|
Hi Adrien did you implement the new mapper? Can you give us a feedback. thanks Nadine
|
| Comment by Nadine Franzen [ 29/Jan/20 ] |
|
any news Adrien. |
| Comment by Adrien Manzoni [ 29/Jan/20 ] |
|
Hi Nadine, I'll work on it today
|
| Comment by Adrien Manzoni [ 29/Jan/20 ] |
|
Hi Riste, Could you please try this <dependency> <groupId>info.magnolia.seo</groupId> <artifactId>magnolia-siteimprove</artifactId> <version>1.1.5-SNAPSHOT</version> </dependency> Just change the following property: /modules/site-improve-connector/config@mapperClass with the following value info.magnolia.connector.siteimprove.mapper.DefaultMapper Let me know how it goes. I'll release it when everything will be fine |
| Comment by Riste Drangovski [ 05/Feb/20 ] |
|
Hi Adrien, everything works as expected now, thanks for your effort. |
| Comment by Adrien Manzoni [ 05/Feb/20 ] |
|
Hi Riste, Good to read that You can now use the following released version: |
| Comment by Adrien Manzoni [ 05/Feb/20 ] |
|
<dependency> |