juliocamarero/liferay-portal — pull request #1884 by lipusz (2013-05-31 15:33:28+00:00)
Title: LPS-34261 Fix API change as we discussed via email

Hi Julio,

Here I'm sending a pull with the changes we discussed via email:

{quote}
Hi Tibor

1. Agree on everything you said for trunk

2. 6.1.x I agree too, let's keep the old method doing what it used to do and have the new one to create the default vocabulary.

thanks!
{/quote}

{quote}
Hi Julio,

I know, it's been a long time since you have reviewed this pull https://github.com/juliocamarero/liferay-portal-ee/pull/4, but recently I've had to re-send some missing changes to ee-6.1.x, in order to fix ihttp://issues.liferay.com/browse/LPS-34261 properly.

However, it seems we introduced some undocumented & unexpected change in the remote service impl. Let me explain.

@trunk
- AVSI#getJSONGroupVocabularies was deprecated
- AVSI#getGroupVocabulariesDisplay was introduced as the new remote API that is called within the asset_categories_admin/js/main.js#_getVocabularies to fetch Group vocabularies.
- AVLSI#addDefaultVocabulary was created by reusing the existing logic of #getGroupVocabularies(long groupId, boolean addDefaultVocabulary)

The old #getJSONGroupVocabularies never leaded to create a default Vocabulary in 6.1, so I think, the new one should keep this backward compatibility somehow.

Thus, we should
1. create a new method called #getGroupVocabulariesDisplay(long groupId, String name, int start, int end, OrderByComparator obc, boolean addDefault)
2. Modify the existing #getGroupVocabulariesDisplay to call this new one with false
3. Modify the main.js to pass true as a new parameter in the service request, so LPS-34261 would remain fixed on trunk.

@ee-6.1.x
It's a different story. Despite the local & remote services have been updated, the Categories Admin portlet (and it's JS) are still missing some backports.
The JS still calls #getJSONGroupVocabularies, but it was changed (by me yesterday) to always create a def. V, when the result list is empty. But, if we release 6.1.30 EE GA3 with this new remote API, there can be customers who relied on the old behavior (that never created a default V).

So, we should
1. create a new method called #getGroupVocabulariesDisplay(long groupId, String name, int start, int end, OrderByComparator obc, boolean addDefault) (sync with trunk)
2. create a new method called #getJSONGroupVocabularies(long groupId, String name, int start, int end, OrderByComparator obc, boolean addDefault) ??? (should it be committed to trunk, too?)
3. Modify the old #getJSONGroupVocabularies to call its new one with false
4. Modify the main.js to pass true as a new parameter in the service request, so LPS-34261 would remain fixed.

{/quote}

Total diff of my changes: https://github.com/lipusz/liferay-portal/compare/2a4501cf63e548e9f59c75bed56f3d96f1517145...102f0a8bd07aa5eaa742bee5d4cfc0a898449d99

Thank you for reviewing! Have a good weekend!

Regards,
Tibor