Skip to content

LPD-69423 highlight sidebar menu correctly #6698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

LPD-69423 highlight sidebar menu correctly #6698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ba5a33
LPD-69423 Handle ObjectEntryFolder and resolve missing DepotEntry in …
robertoDiaz Oct 24, 2025
46b7aa9
LPD-69423 Add integration test for verifying props method use case in…
robertoDiaz Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions ...s/site/initializer/internal/display/context/test/ViewSpacesSectionDisplayContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,24 @@
import com.liferay.fragment.renderer.FragmentRenderer;
import com.liferay.headless.asset.library.dto.v1_0.AssetLibrary;
import com.liferay.headless.asset.library.resource.v1_0.AssetLibraryResource;
import com.liferay.object.constants.ObjectEntryFolderConstants;
import com.liferay.object.model.ObjectEntryFolder;
import com.liferay.object.service.ObjectEntryFolderLocalService;
import com.liferay.portal.kernel.json.JSONArray;
import com.liferay.portal.kernel.json.JSONObject;
import com.liferay.portal.kernel.model.User;
import com.liferay.portal.kernel.security.permission.PermissionThreadLocal;
import com.liferay.portal.kernel.service.ServiceContext;
import com.liferay.portal.kernel.test.ReflectionTestUtil;
import com.liferay.portal.kernel.test.rule.AggregateTestRule;
import com.liferay.portal.kernel.test.util.RandomTestUtil;
import com.liferay.portal.kernel.test.util.ServiceContextTestUtil;
import com.liferay.portal.kernel.test.util.TestPropsValues;
import com.liferay.portal.kernel.test.util.UserTestUtil;
import com.liferay.portal.kernel.theme.ThemeDisplay;
import com.liferay.portal.kernel.util.HashMapBuilder;
import com.liferay.portal.kernel.util.LocaleUtil;
import com.liferay.portal.kernel.util.StringUtil;
import com.liferay.portal.kernel.util.WebKeys;
import com.liferay.portal.test.rule.FeatureFlag;
import com.liferay.portal.test.rule.FeatureFlags;
Expand All @@ -34,6 +42,8 @@

import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;

import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -188,6 +198,52 @@ public void testGetPage() throws Exception {
Assert.assertTrue(items.contains(pinnedByMeAssetLibrary5));
}

@Test
public void testGetProps() throws Exception {
AssetLibrary assetLibrary1 = _addAssetLibrary();

_addAssetLibrary();

DepotEntry depotEntry = _depotEntryLocalService.getDepotEntry(
assetLibrary1.getId());

ObjectEntryFolder objectEntryFolder =
_objectEntryFolderLocalService.addObjectEntryFolder(
null, depotEntry.getGroupId(), TestPropsValues.getUserId(),
ObjectEntryFolderConstants.
PARENT_OBJECT_ENTRY_FOLDER_ID_DEFAULT,
RandomTestUtil.randomString(),
HashMapBuilder.put(
LocaleUtil.getDefault(), StringUtil.randomString()
).build(),
StringUtil.randomString(),
ServiceContextTestUtil.getServiceContext());

Map<String, Object> props = ReflectionTestUtil.invoke(
_getViewSpacesDisplayContext(
getMockHttpServletRequest(objectEntryFolder)),
"getProps", new Class<?>[0]);

Assert.assertNotNull(props);

JSONArray assetLibrariesJSONArray = (JSONArray)props.get(
"assetLibraries");

for (int i = 0; i < assetLibrariesJSONArray.length(); i++) {
JSONObject assetLibraryJSONObject =
assetLibrariesJSONArray.getJSONObject(i);

if (Objects.equals(
assetLibraryJSONObject.get("id"), assetLibrary1.getId())) {

Assert.assertTrue(assetLibraryJSONObject.getBoolean("active"));
}
else {
Assert.assertFalse(assetLibraryJSONObject.getBoolean("active"));
}
}
}
Comment on lines +201 to +245
Copy link

@gemini-code-assist gemini-code-assist bot Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The test method testGetProps lacks proper cleanup after execution. Specifically, the ObjectEntryFolder created within the test is not deleted, which can lead to data pollution and potential interference with other tests. This can cause intermittent test failures and make the test suite unreliable.

Consider adding code to delete the created ObjectEntryFolder in a finally block or using a @After annotated method to ensure cleanup even if the test fails.

    @Test
    public void testGetProps() throws Exception {
        AssetLibrary assetLibrary1 = _addAssetLibrary();

        _addAssetLibrary();

        DepotEntry depotEntry = _depotEntryLocalService.getDepotEntry(
            assetLibrary1.getId());

        ObjectEntryFolder objectEntryFolder = null;

        try {
            objectEntryFolder =
                _objectEntryFolderLocalService.addObjectEntryFolder(
                    null, depotEntry.getGroupId(), TestPropsValues.getUserId(),
                    ObjectEntryFolderConstants.
                        PARENT_OBJECT_ENTRY_FOLDER_ID_DEFAULT,
                    RandomTestUtil.randomString(),
                    HashMapBuilder.put(
                        LocaleUtil.getDefault(), StringUtil.randomString()
                    ).build(),
                    StringUtil.randomString(),
                    ServiceContextTestUtil.getServiceContext());

            Map<String, Object> props = ReflectionTestUtil.invoke(
                _getViewSpacesDisplayContext(
                    getMockHttpServletRequest(objectEntryFolder)),
                "getProps", new Class<?>[0]);

            Assert.assertNotNull(props);

            JSONArray assetLibrariesJSONArray = (JSONArray)props.get(
                "assetLibraries");

            for (int i = 0; i < assetLibrariesJSONArray.length(); i++) {
                JSONObject assetLibraryJSONObject =
                    assetLibrariesJSONArray.getJSONObject(i);

                if (Objects.equals(
                        assetLibraryJSONObject.get("id"), assetLibrary1.getId())) {

                    Assert.assertTrue(assetLibraryJSONObject.getBoolean("active"));
                }
                else {
                    Assert.assertFalse(assetLibraryJSONObject.getBoolean("active"));
                }
            }
        } finally {
            if (objectEntryFolder != null) {
                _objectEntryFolderLocalService.deleteObjectEntryFolder(objectEntryFolder);
            }
        }
    }


protected ThemeDisplay getUserThemeDisplay(
HttpServletRequest httpServletRequest, User user)
throws Exception {
Expand Down Expand Up @@ -286,4 +342,7 @@ private Object _getViewSpacesDisplayContext(
)
private FragmentRenderer _fragmentRenderer;

@Inject
private ObjectEntryFolderLocalService _objectEntryFolderLocalService;

}
15 changes: 15 additions & 0 deletions ...lizer/src/main/java/com/liferay/site/cms/site/initializer/internal/util/InfoItemUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package com.liferay.site.cms.site.initializer.internal.util;

import com.liferay.depot.model.DepotEntry;
import com.liferay.depot.service.DepotEntryLocalServiceUtil;
import com.liferay.info.constants.InfoDisplayWebKeys;
import com.liferay.object.model.ObjectEntry;
import com.liferay.object.model.ObjectEntryFolder;

import jakarta.servlet.http.HttpServletRequest;

Expand All @@ -23,6 +25,11 @@ public static long getDepotEntryId(HttpServletRequest httpServletRequest) {
DepotEntry depotEntry =
object instanceof DepotEntry ? (DepotEntry)object : null;

if (depotEntry == null) {
depotEntry = DepotEntryLocalServiceUtil.fetchGroupDepotEntry(
getGroupId(httpServletRequest));
Comment on lines +28 to +30
Copy link

@gemini-code-assist gemini-code-assist bot Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The fetchGroupDepotEntry method is used without proper error handling. If fetchGroupDepotEntry returns null, the code will continue without a valid depotEntry, potentially leading to a NullPointerException later on. It's crucial to handle the case where no DepotEntry is found for the group.

Consider adding a check for null after calling fetchGroupDepotEntry and either logging an error or throwing an exception to prevent further execution with invalid data.

        if (depotEntry == null) {
            depotEntry = DepotEntryLocalServiceUtil.fetchGroupDepotEntry(
                getGroupId(httpServletRequest));

            if (depotEntry == null) {
                // Log an error or throw an exception
                // Example: 
                // throw new Exception("No DepotEntry found for group " + getGroupId(httpServletRequest));
                return 0; // Or throw an exception
            }
        }

}

if (depotEntry != null) {
return depotEntry.getDepotEntryId();
}
Expand All @@ -48,6 +55,14 @@ public static long getGroupId(HttpServletRequest httpServletRequest) {
return objectEntry.getGroupId();
}

ObjectEntryFolder objectEntryFolder =
object instanceof ObjectEntryFolder ? (ObjectEntryFolder)object :
null;

if (objectEntryFolder != null) {
return objectEntryFolder.getGroupId();
}

return 0;
}

Expand Down