adobe / aem-sample-we-retail-journal

We.Retail Journal is a sample showcasing SPA Editing capabilities in AEM using React and Angular
Apache License 2.0
70 stars 69 forks source link

Injecting currentPage using annotations does not return the correct page #53

Closed cqsapient closed 4 years ago

cqsapient commented 5 years ago

None of these annotations when used in a sling model in this project -

 @Inject @Source("script-bindings")
    Page currentPage;

or

@ScriptVariable
    private Page currentPage;

or

@Inject
    private Page currentPage;

return the correct currentPage - these always return the last created page. Not sure what's the reason behind this -

Using resourcePage works;

@Inject
    private Page resourcePage;

but it will not be helpful in the cases where the component is locked in the structure of the template, and hence is not inside the jcr:content of the page, in those cases resourcePage returns the template's path as the component(say header) is inside the template.

Could someone please look into this as this is a major issue and once should be able to inject OOTB sling binding variables.

godanny86 commented 5 years ago

Agreed, @cqsapient I have seen inconsistencies with resolving the "currentPage" with the SPA implementation. I believe this is due to how the JSON gets generated via the HiearchyPageImpl model. The JSON gets collected for multiple depths but in cases like this the correct context gets lost.

Question for you, if you request the exact path of your page i.e /content/spa/root/mypage.model.json vs /content/spa/root.model.json do you get the expected value?

cqsapient commented 5 years ago

Thanks for replying @godanny86

Yes, as you mentioned, the issue is only in /content/spa/root.model.json

when using the .model selector in any other page; say /content/spa/root/mypage.model.json

currentPage is properly initialized.

This is because HiearchyPageImpl model comes into picture only when root.model.json is requested; I tried debugging it and found that the debugger does not go to HiearchyPageImpl when .model json is appended in any other page. I know that HiearchyPageImpl is causing this issue; it is setting the attribute -CURRENT_PAGE_ATTR on the wrapper SlingRequest in these lines.

wrapper.setAttribute(COMPONENT_CONTEXT_ATTR, new HierarchyComponentContextWrapper(componentContext, hierarchyPage));
        wrapper.setAttribute(CURRENT_PAGE_ATTR, hierarchyPage);

I tried commenting these lines and deployed the code; but still it did not fix the issue.

It will be helpful that the engineering team of AEM SPA module looks into it and provides a resolution so that we are able to utilize all capabilities of the powerful editable templates feature of AEM sites; as it is not possible to use currentPage if the component needs to be locked down in the template.

pfauchere commented 5 years ago

Hi, by default the referenced page used by the injector is only set once per request from the entry point. If your entry point is /content/spa/root.model.json, the resource relative to root will be the current page. As in the context of the HiearchyPageImpl we want to list a tree of pages, we updated the ComponentContext with the expected current page. This workaround could be avoided if we improve or provide a dedicated injector

My questions to you are:

ergo123 commented 5 years ago

I have a similar issue. I'm trying to write react version of Breadcrumb component from core components and for each page the number of items is the same. It looks like the breadcrumb items are being generated only for the last page in :children property from root model.json. I believe this is the 'last created page'.

I debugged the hierarchyPage and it changes accordingly for each page, but when I had a look at currentPage in BreadrumbImpl model, the value was not the expected one.

A quick workaround could be

currentPage = request.getResourceResolver().adaptTo(PageManager.class).getContainingPage(resource)

instead of

    @ScriptVariable
    private Page currentPage;
pfauchere commented 4 years ago

I recall having to work around that very same issue. That said, the problem relates neither to the current project nor the SPA Editor feature but to the overall AEM/Sling Dependency Injection and Binding Framework. Would be best to open a ticket either via AEM Customer Care or Sling depending on the assumption as to where the issue is located. I have to admit that I haven't investigated the location of the underlying issue

GonzaloCalandria commented 3 years ago

Was this fixed? I'm still having breadcrumb issue with currentPage.

niekraaijmakers commented 3 years ago

I believe we should use resourcePage for this context. That is much more robust then currentPage.

Of course that requires ACS aem commons I believe ...

GonzaloCalandria commented 2 years ago

@niekraaijmakers what do you mean by using ACS aem commons? any news on this issue? I don't understand why this issue got closed... this is a very important issue.. many clients use a breadcrumb at template level.