Skip to content

Conversation

@juanjofgliferay
Copy link
Collaborator

@juanjofgliferay juanjofgliferay commented Oct 31, 2025

Story / Task

LPD-9465 / LPD-59387

Warning

In the FDS Sample Custom Views, Config in URL and Active View Settings may clash and return unexpected results.
If you want to test Custom/User Views in the FDS Sample it is better to start with a fresh DB or remove the corresponding ActiveViewSettingsJSON rows from the PortalPreferenceValue table.

Tip

To test this feature we need to update a given Data Set and set customViewsEnabled to true. This can be done throug API Explorer using the Data Sets API /by-external-reference-code/{externalReferenceCode} PATCH with {"snapshotsEnabled": true} body.
To ease testing, it is possible to enable custom views via UI with LPD-10683 feature flag turned on.

Goal

Allows end-users to create, edit and delete customized versions of admin views.
Aspects that can be saved as part of the custom view:

  • Visualization Mode
  • To be done in next Story:
  • Pagination delta
  • Filters applied
  • Current sorting
  • Visualization Mode
  • Columns shown

UI

Screencast.From.2025-11-20.13-01-15.mp4

Tests

FDS web

fds-web-test

FDS admin web

fds-admin-web-test

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

Test suite sf has been triggered on http://test-1-36

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 5 minutes

Ran com.liferay.source.formatter at released version 1.0.1548.

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 3dfb64776ee5a5798ad4a1c9138b1472847ed139

Sender Branch:

Branch Name: LPD-9465
Branch GIT ID: eba6cdaa1d0f73535ae789eadb51cae469ac3e2c

1 out of 1 jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

"system": true,
"type": "String"
},
{
Copy link
Collaborator Author

@juanjofgliferay juanjofgliferay Oct 31, 2025

Choose a reason for hiding this comment

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

I've started with a simple object model, as @dsanz suggest. userId field is missing as I'm expecting to filter entries in the backend using the objects creator information.
Another missing part is a field to differentiate between system and custom views. Maybe a custom boolean field can be enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

why would you need to differentiate between system and custom views?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technical notes mention it.

Data Set Manager persistence should be used to store the data. We need to make sure we make a distinction between “system views” and “custom views”

).build()
%>'
apiURL="<%= fdsSampleDisplayContext.getAPIURL() %>"
customViewsEnabled="<%= true %>"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove customViews from FDS Sample as /o/fdssamples does not provide a way to store this information.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This signals a key decision in our solution. The fact that there is no "DataSet" object entry associated with the custom view should not prevent custom views feature from being exercised.

I see 2 options here:

  • Create a DataSet entry on the fly when saving a custom view, if it does not exist. It could even be created in a deactivated state. Associate custom view with it.
  • Make custom view object definition a totally independent entity, with no connection with DataSet object definition.

First option brings other important issues, namely:

  • Shall it import the entire DS into DSM (filters, actions, views, etc)? If the DS is a system DS, then perhaps, but if it's an old, traditional, java-based DS, the import process would be lossy
  • It'll be not possible to distinguish these imported DS from normal "custom" data sets because the plain, old, java-based DS are not in the system FDS registry, which we use to render the lists of custom/system data sets in the DSM
  • Management of deletions (system data sets): for system data sets, deleting the editable copy (DS object entry) brings the DS to its default state. But custom views shall stay. For custom data sets, this option manages things pretty well as custom views would be gone along with the custom data set entry.

Second option does not come without issues either:

  • Management of deletions: just the opposite of the above. System data sets are fine with this, as we want custom views to remain. But custom data sets need a more ellaborated deletion logic that would go in a dedicated FDSAdmin portlet action in charge of deleting the associated custom views.
  • Filtering capabilities: this isolated object definition has specific filtering requirements for it to be viable solution
    • we'll need to manage this big set of custom views from DSM UI at some point. This brings the issue of filtering the items, e.g. show the custom views associated to the DS I am managing in the DSM. This should not be an issue but it depends on filtering requirements.
    • basic rendering and FDS-managed abilities related to custom views: serialization of object entries representing custom views, retrieving the list of custom views for an user/portlet/fragment/DS id, etc

So, I'd lean towards second option as issues with first one seem more important and difficult to solve

else {
Boolean customViewsEnabled;

if (Validator.isNull(props.get("customViewsEnabled"))) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably there are better ways to check and cast this type of values 🤷

}

@Consumes(MediaType.APPLICATION_JSON)
@Path("/data-set/{id}/save-active-view-settings")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This save-active-view-settings is triggered multiple times in the FDS (with CustomViews enabled) and it is related with some FrontendDataSet.tsx (saveViewSettings.js) code that maybe is worth removing. Not sure about this point.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can migrate this logic afterwards. It's not so immediate: active view settings are scoped to the portletId, whereas object-persisted custom views are not. This means that same fdsName placed on 2 different pages would keep 2 separate sets of active view settings for the same user.

Ideally, we shall remove this entirely once we reach a solution to persist custom views.

Then we will leverage custom views backend to persist active view settings. It would be a sort of "anonymous", special, custom view that, similar to config-in-url feature, flushes relevant , current component state in the backend as it changes. Note that "regular" custom views are saved only when user hits the save button.

I called this custom view "special" because it has specific treatment:

  • It has no name, at least, user will not provide a name
  • It is saved under the hoods, proactively, when FDS changes its state, very much like URL gets updated automatically
  • It is never loaded in the dropdown

All this shall be done later on, in a separate Epic, unless we clearly see it possible with a small effort :trollface:

return {
...state,
pageNumber: value,
viewUpdated: false,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove this property as it was causing problems when running a BATCH_UPDATE. x.e. changing UPDATE_PAGINATION_DELTA updates the view and after that the UPDATE_PAGE_NUMBER does not update it, so we lose the * from the custom view dropdown.

Copy link
Collaborator

Choose a reason for hiding this comment

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

mmm interesting side effect. If we are not saving pageNumber as part of custom view data, then changing page number shall not show the *. When changing the delta, however, it should.

So, a batch update shall apply the updates and if one of it returns viewUpdated as true, then the entire batch shall be considered as a view update and so keep the asterisk

Copy link
Collaborator

@kresimir-coko kresimir-coko left a comment

Choose a reason for hiding this comment

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

small nits, no red flags but out of my scope definitely with all the Java changes 😅

Copy link
Collaborator

@dsanz dsanz left a comment

Choose a reason for hiding this comment

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

Just some comments to start conversation. I have not reviewed the tests yet as it looks fair enough to begin with

}

@Consumes(MediaType.APPLICATION_JSON)
@Path("/data-set/{id}/save-active-view-settings")
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can migrate this logic afterwards. It's not so immediate: active view settings are scoped to the portletId, whereas object-persisted custom views are not. This means that same fdsName placed on 2 different pages would keep 2 separate sets of active view settings for the same user.

Ideally, we shall remove this entirely once we reach a solution to persist custom views.

Then we will leverage custom views backend to persist active view settings. It would be a sort of "anonymous", special, custom view that, similar to config-in-url feature, flushes relevant , current component state in the backend as it changes. Note that "regular" custom views are saved only when user hits the save button.

I called this custom view "special" because it has specific treatment:

  • It has no name, at least, user will not provide a name
  • It is saved under the hoods, proactively, when FDS changes its state, very much like URL gets updated automatically
  • It is never loaded in the dropdown

All this shall be done later on, in a separate Epic, unless we clearly see it possible with a small effort :trollface:

() -> {
JSONArray customViewsJSONArray =
fdsSerializer.serializeCustomViews(
fdsName, httpServletRequest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might not need to serialize custom views. We can do that once component renders. I'm fine with this approach too for the first render, but we shall consider loading them on the fly too, after render

Just consider the case I'm trying to avoid if we only serialize custom views on first render:

  • User saves a new custom view
  • Then, in order for that custom view to be displayed, we need to reload the entire page where FDS is, or manipulate the custom views dropdown frontend component programmatically
  • Then user deletes a custom view (same case)

).build()
%>'
apiURL="<%= fdsSampleDisplayContext.getAPIURL() %>"
customViewsEnabled="<%= true %>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This signals a key decision in our solution. The fact that there is no "DataSet" object entry associated with the custom view should not prevent custom views feature from being exercised.

I see 2 options here:

  • Create a DataSet entry on the fly when saving a custom view, if it does not exist. It could even be created in a deactivated state. Associate custom view with it.
  • Make custom view object definition a totally independent entity, with no connection with DataSet object definition.

First option brings other important issues, namely:

  • Shall it import the entire DS into DSM (filters, actions, views, etc)? If the DS is a system DS, then perhaps, but if it's an old, traditional, java-based DS, the import process would be lossy
  • It'll be not possible to distinguish these imported DS from normal "custom" data sets because the plain, old, java-based DS are not in the system FDS registry, which we use to render the lists of custom/system data sets in the DSM
  • Management of deletions (system data sets): for system data sets, deleting the editable copy (DS object entry) brings the DS to its default state. But custom views shall stay. For custom data sets, this option manages things pretty well as custom views would be gone along with the custom data set entry.

Second option does not come without issues either:

  • Management of deletions: just the opposite of the above. System data sets are fine with this, as we want custom views to remain. But custom data sets need a more ellaborated deletion logic that would go in a dedicated FDSAdmin portlet action in charge of deleting the associated custom views.
  • Filtering capabilities: this isolated object definition has specific filtering requirements for it to be viable solution
    • we'll need to manage this big set of custom views from DSM UI at some point. This brings the issue of filtering the items, e.g. show the custom views associated to the DS I am managing in the DSM. This should not be an issue but it depends on filtering requirements.
    • basic rendering and FDS-managed abilities related to custom views: serialization of object entries representing custom views, retrieving the list of custom views for an user/portlet/fragment/DS id, etc

So, I'd lean towards second option as issues with first one seem more important and difficult to solve

"customViewsEnabled",
properties.get("customViewsEnabled")
).put(
"dataSetERC", externalReferenceCode
Copy link
Collaborator

Choose a reason for hiding this comment

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

id identifies the data set we are rendering. If id is not passed for some reason, then custom views shall not be enabled in the component. Therefore, passing the ERC shall not be necessary as FDSRenderer already handles that

It just happens that serializing a DS persisted in object uses the DS object entry ERC as fdsName , whereas Java-based DS use the provided tag id attribute for the same purpose

return customViewsJSONArray;
}
).put(
"customViewsEnabled", customViewsEnabled
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor nit: usually, code that computes the value is put here, in a lambda expression. Curiously, if we need the value outside the lambda we compute it in advance, and this could be the case if we decide to serialize custom views: if flag is off, we won't serialize them

"system": true,
"type": "String"
},
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

why would you need to differentiate between system and custom views?

}
).put(
"customViews", _getCustomViews()
"customViews", new ArrayList<String>()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small nit. Here we can take data from the serializer as well. Once we get rid of system data sets FF, this branch will be gone and everything will be managed by FDSRenderer

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Apply same changes as CustomFDSSerializer

return {
...state,
pageNumber: value,
viewUpdated: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

mmm interesting side effect. If we are not saving pageNumber as part of custom view data, then changing page number shall not show the *. When changing the delta, however, it should.

So, a batch update shall apply the updates and if one of it returns viewUpdated as true, then the entire batch shall be considered as a view update and so keep the asterisk

custom-user-attributes-help=Specifying the custom user attributes retrieves assets that have matching categorization. Categories must be from the global context.
custom-user-mapping=Custom User Mapping
custom-view=Custom View
custom-view-name-updated={0} Updated
Copy link
Collaborator

Choose a reason for hiding this comment

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

This pattern looks odd to me. Usually there is some key like x-was-updated-successfully that we can fill in with a title or the custom view name

Copy link
Collaborator

@dsanz dsanz left a comment

Choose a reason for hiding this comment

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

Some spare comments, will continue reviewing if I have a chance tomorrow

String filterExpression = StringBundler.concat(
"'fdsName' eq '", fdsName, "' and 'creatorId' eq '",
themeDisplay.getUserId(), "'");

Copy link
Collaborator

Choose a reason for hiding this comment

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

if _serializeCustomViews() has the same logic in both serializers, then we need to hoist up this method to a the parent BaseFDSSerializer abstract class.

Method in the Base class should be protected and be named serializeCustomViews (w/o the _ as it's no longer private).

You'll need also to hoist up _objectDefinitionLocalService into a protected attribute.

You can use what's already in the Base class as reference. We can checkup together in friday if needed

WebKeys.THEME_DISPLAY);

if (Validator.isNull(props.get("customViewsEnabled")) &&
themeDisplay.isSignedIn()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the expected behavior when props.get("customViewsEnabled") returns true for a dataset and user is not signed in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No Custom/User Views will be available. We need the FDSName and userId to store de view.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, nevertheless, the else clause is relying on the snapshotsEnabled setting value, which may be "true"

size={1}
>
<ClayToggle
id="user-custom-views"
Copy link
Collaborator

Choose a reason for hiding this comment

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

user-custom-views looks confusing, we can use user-views-toggle or similar

@juanjofgliferay
Copy link
Collaborator Author

ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

Test suite sf has been triggered on http://test-1-35

* @returns A deep clone of the object.
*/
export default function deepClone(object: any) {
if (object === null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@markocikos Minor change to handle null. Otherwise it will be treated typeof object === 'object'

@juanjofgliferay
Copy link
Collaborator Author

ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

Test suite sf has been triggered on http://test-1-37

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:sf - 0 out of 1 jobs passed in 5 minutes

Ran com.liferay.source.formatter at released version 1.0.1554.
*The latest version has not been released.

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 10523f8459d048a14619b81344c75208b37d6815

Sender Branch:

Branch Name: LPD-9465
Branch GIT ID: 7dfd55758bc73c6ebaf25eca113ac509744dbbfc

0 out of 1 jobs PASSED
For more details click here.
     [exec] > Task :packageRunCheckFormat
     [exec] yarn run v1.22.22
     [exec] \$ node-scripts check:ci
     [exec] 
     [exec] ⚙️ Running preflight checks...
     [exec] 
     [exec] ⚙️ Checking outdated tsconfig.json files ...
     [exec] 
     [exec] ⚙️ Running TypeScript checks on modified files...
     [exec] ℹ️ A total of 12 CPUs were detected: launching tsc using 12 workers
     [exec] ✅ Checked apps/frontend-js/frontend-js-web
     [exec] ✅ Checked apps/frontend-data-set/frontend-data-set-sample-web
     [exec] ✅ Checked apps/frontend-data-set/frontend-data-set-web
     [exec] ✅ Checked apps/frontend-data-set/frontend-data-set-admin-web
     [exec] 
     [exec] ⚙️ Running format checks on modified files...
     [exec]    /opt/dev/projects/github/liferay-portal/modules/apps/frontend-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/FrontendDataSet.tsx
     [exec]      522:3  error  React Compiler has skipped optimizing this component because one or more React ESLint rules were disabled. React Compiler only works when your components follow all the rules of React, disabling them may result in unexpected or incorrect behavior  react-compiler/react-compiler
     [exec]    
     [exec]    ✖ 1 problem (1 error, 0 warnings)
     [exec]    
     [exec]    
     [exec] ❌ CI checks failed.
     [exec] 
     [exec] 
     [exec] FAILURE: Build failed with an exception.
     [exec] 
     [exec] * What went wrong:
     [exec] Execution failed for task ':packageRunCheckFormat'.
     [exec] > Process 'command '/opt/dev/projects/github/liferay-portal/build/node/bin/node'' finished with non-zero exit value 1
     [exec] 
     [exec] * Try:
     [exec] > Run with --info or --debug option to get more log output.
     [exec] > Run with --scan to get full insights.
     [exec] > Get more help at https://help.gradle.org.
     [exec] 
     [exec] * Exception is:
     [exec] org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':packageRunCheckFormat'.
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda\$executeIfValid\$1(ExecuteActionsTaskExecuter.java:148)
     [exec]   at org.gradle.internal.Try\$Failure.ifSuccessfulOrElse(Try.java:282)
     [exec] error Command failed with exit code 1.  at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:146)

@liferay-continuous-integration
Copy link
Collaborator

(!filterClientExtensionDefinitions.length &&
!cellClientExtensionDefinitions.length) ||
cellClientExtensionsLoaded ||
filterClientExtensionsLoaded
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@markocikos This is the place where we need to check for the filterClientExtensionsLoaded to avoid the infinite loop due to the filters dependency.

@juanjofgliferay
Copy link
Collaborator Author

ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

Test suite sf has been triggered on http://test-1-39

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 5 minutes

Ran com.liferay.source.formatter at released version 1.0.1554.
*The latest version has not been released.

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ffb154d7f2fd1d9740ece84e9aac5863c9e50cee

Sender Branch:

Branch Name: LPD-9465
Branch GIT ID: 36fd3ec4ab5a6754b76b6cad6b9cf396ab4c5b60

1 out of 1 jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants