Use site for loading of actions via Xhr and allow client specifying view #1169
Conversation
| @@ -99,13 +122,28 @@ define(["dojo/_base/declare", | |||
| var nodeRef = lang.getObject("nodeRef", false, this.currentItem); | |||
| if (nodeRef) | |||
| { | |||
| // depending on how item was loaded site can be coded in different ways | |||
| var site; | |||
| if (this.currentItemSiteShortNameKey) | |||
There was a problem hiding this comment.
This configuration option was put in place to allow short-circuit of the "default lookup chain" if a developer is sure enough about how site can be extracted from the current item.
| * | ||
| * @instance | ||
| * @type {string} | ||
| * @default null |
There was a problem hiding this comment.
When the initial assignment is null, a String, number or a boolean it is not necessary to set it on the default annotation... JSDoc automatically figures that out.
|
Thanks for raising this... I've made a few initial comments and I'll checkout your branch and run the tests locally to ensure there are no failures. |
|
Requested changes have been made to PR branch. |
|
Thanks for making those changes - I'm running the unit tests and hitting some issues, I'll add an update when I've got to the bottom of what the problems/failures are. |
|
I've tested the changes on Share and can't find any issues, I think it might be something to do with the mock data provided for the unit tests, the URLs must have changed somehow - I'll dig further into it. |
| @@ -329,7 +325,12 @@ define(["dojo/_base/declare", | |||
| } | |||
| else | |||
| { | |||
| url = AlfConstants.URL_SERVICECONTEXT + "components/documentlibrary/data/doclist/" + params; | |||
| url = AlfConstants.URL_SERVICECONTEXT + "components/documentlibrary/data/"; | |||
| if (payload.site) | |||
There was a problem hiding this comment.
It looks like this if block is not required. Stepping through the code in the unit test it seems that we're getting the site data included twice, for example on the "develop" branch we see this request:
/aikau/service/components/documentlibrary/data/doclist/all/site/site1/documentlibrary?filter=path&size=25&pos=1&sortAsc=false&sortField=cm%3Aname&view=browse&noCache=1470142533664
...but on your branch we see this:
/aikau/service/components/documentlibrary/data/site/site1/doclist/all/site/site1/documentlibrary?filter=path&size=25&pos=1&sortAsc=false&sortField=cm%3Aname&view=browse&noCache=1470142609664
Note the additional "site/site1" after "data" and before "doclist". Any thoughts on this?
I'll continue to test my update against Share to see if it causes any issues.
There was a problem hiding this comment.
The fragment until (and including) data/site/site1 is consumed by the Share data web script surf-doclist.get (URL pattern /components/documentlibrary/data/site/{site}/{webscript}/{params}) while the remainder is captured as the repository web script to call. As a result, the site is contained twice if both tiers are to function correctly. Otherwise the Share-tier won't capture the site token for use in any evaluators (e.g. Surf extension modules).
The if block is the integral point of this PR.
There was a problem hiding this comment.
OK... I hadn't realised that. Well, it was working in Share, so I guess that makes sense... in which case, it will be necessary to push some changes to the mocking service (aikau/src/test/resources/testApp/js/aikau/testing/mockservices/FullDocLibMockXhr.j).
In the setupServer function the first call to this.server.respondWith needs to be updated.
So...
/\/aikau\/service\/components\/documentlibrary\/data\/doclist\/all\/site\/site1\/documentlibrary\?filter=path(.*)/,...needs to become...
/\/aikau\/service\/components\/documentlibrary\/data\/site\/site1\/doclist\/all\/site\/site1\/documentlibrary\?filter=path(.*)/,There was a problem hiding this comment.
The last call (for favourites) will also need updating!
|
I think if you remove that |
|
MERGED ! Thanks for your patience with this one and thanks for the contribution! |
When using the XhrActions module (as it is done in the faceted search page) some actions may not be listed as "available" if
This is due to the fact that the code for handling XhrActions and retrieval of single document data is not handling the site of the document / item.
This PR addresses the issue above by adding site handling to the XhrActionsMixin and the DocumentService modules to ensure the call to the Share-tier data web script is passed the specific site of the document / item. Additionally, it allows a client module to request a specific view instead of the default "browse" (e.g. if actions are associated with a different action group than XY-browse, i.e. XY-details).