Skip to content

Conversation

@thektan
Copy link
Collaborator

@thektan thektan commented Oct 14, 2025

Bug Ticket: https://liferay.atlassian.net/browse/LPD-68453


This fixes the clear filter buttons (in the active filters bar/empty state) to clear the pre-loaded filter. This was previously mutating the object so the component wasn't re-rendering so now it returns a new object. This was also how the logic worked before.

Previously the dispatch looked like:

		viewsDispatch({
			type: VIEWS_ACTION_TYPES.UPDATE_FILTERS,
			value: filters.map((filter) => ({
				...filter,
				active: false,
				odataFilterString: undefined,
				selectedData: undefined,
			})),
		});

When a filter is active on initial load due to `preloadedData`, clicking the "Clear Filters" button fails to clear the filter from the UI if the user has not interacted with it first.

The `deactivateFilter` utility function was directly mutating the filter object, which prevented React from detecting a state change and re-rendering the UI. The function is now updated to return a new, immutable object, ensuring React sees the change and updates the UI correctly.
@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-31

@liferay-continuous-integration
Copy link
Collaborator

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

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

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 25cca0d75ec64ed6265f8101d9c9e9296e5bd620

Sender Branch:

Branch Name: LPD-68453_FixFilterClear
Branch GIT ID: cf5eba1e8af86a565652c1ffbda0ec8d0c4dd4a8

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

@liferay-continuous-integration
Copy link
Collaborator

Jenkins Report:jenkins-report.html
Jenkins Suite:sf
Testray Routine:EE Pull Request
Testray Build ID:338879973

@thektan
Copy link
Collaborator Author

thektan commented Oct 14, 2025

ci:test:frontend-data-set

@liferay-continuous-integration
Copy link
Collaborator

Test suite frontend-data-set has been triggered on http://test-1-25

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:frontend-data-set - 3 out of 6 jobs passed in 56 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 25cca0d75ec64ed6265f8101d9c9e9296e5bd620

Upstream Comparison:

Branch GIT ID: f54453ace69577a7fb5ba5535e9e2ec0f618c787
Jenkins Build URL: EE Development Acceptance (master) - 171 - 2025-10-07[08:39:50]

ci:test:frontend-data-set - 3 out of 6 jobs PASSED
3 Successful Jobs:
    For more details click here.

    Failures unique to this pull:

    For upstream results, click here.

    Test bundle downloads:

    @liferay-continuous-integration
    Copy link
    Collaborator

    @dsanz
    Copy link
    Collaborator

    dsanz commented Oct 15, 2025

    Great catch @thektan , my bad.

    I'd like to review the activateFilter utility as well, because that one does mutate the filter. There is a case (getInitialViewState) where original code mutated the filter and I'd say that's why I wrongly implemented utilities in that way. Here returning a new object is not strictly needed but won't hurt.

    The other usage is a bit more convoluted, I believe this one would benefit from returning a new object. could you please take a deeper look? Thx

    Copy link
    Collaborator

    @markocikos markocikos left a comment

    Choose a reason for hiding this comment

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

    @thektan LGTM! ✨

    @thektan
    Copy link
    Collaborator Author

    thektan commented Oct 16, 2025

    @dsanz I'm going to keep activateFilter the same. I was attempting to make it avoid any mutations, but it resulted in more issues with the filters not even applying anymore. I noticed places require possibly bigger changes like in Filter.tsx where it depends on the mutation happening.

    Also it looks like activateFilter was implemented the same way it was before. With the mutations.

    For example:

    In the filter resume dropdown, if you unchecked all the selections and then clicked "Delete Filter", the filter resume label would still show for example: "Color:".
    
    This is because `setFilter` needs to be able to also set `active: false` but `activateFilter` was always setting `active: true`
    @thektan
    Copy link
    Collaborator Author

    thektan commented Oct 16, 2025

    Added a change to fix the filter resume not fully clearing.

    Kapture.2025-10-16.at.12.08.00.mp4

    Also might be something broken with the forward/back history navigation, but I can't reliably reproduce it. It seems sometimes the forward history gets wiped out when you hit back navigation to the initial state (plus it gets suck there where continually hitting back navigation does nothing). 🙃

    Mainly looking at this test specifically:

    Screenshot 2025-10-16 at 1 13 19 PM

    @thektan
    Copy link
    Collaborator Author

    thektan commented Oct 16, 2025

    ci:test:frontend-data-set

    @liferay-continuous-integration
    Copy link
    Collaborator

    Test suite frontend-data-set has been triggered on http://test-1-34

    @liferay-continuous-integration
    Copy link
    Collaborator

    ❌ ci:test:frontend-data-set - 3 out of 6 jobs passed in 1 hour 6 minutes

    Click here for more details.

    Base Branch:

    Branch Name: master
    Branch GIT ID: 2018e7596b60007f382316cc000ce88d1994c6fa

    Upstream Comparison:

    Branch GIT ID: 6bfad7c5eeca7396b9756980c32197057cc003f2
    Jenkins Build URL: EE Development Acceptance (master) - 179 - 2025-10-10[20:39:27]

    ci:test:frontend-data-set - 3 out of 6 jobs PASSED
    3 Successful Jobs:
      For more details click here.

      Failures unique to this pull:

      For upstream results, click here.

      Test bundle downloads:

      @liferay-continuous-integration
      Copy link
      Collaborator

      @liferay-continuous-integration
      Copy link
      Collaborator

      @thektan
      Copy link
      Collaborator Author

      thektan commented Oct 17, 2025

      ci:test:sf

      @thektan
      Copy link
      Collaborator Author

      thektan commented Oct 17, 2025

      ci:test:relevant

      @liferay-continuous-integration
      Copy link
      Collaborator

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

      @liferay-continuous-integration
      Copy link
      Collaborator

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

      @liferay-continuous-integration
      Copy link
      Collaborator

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

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

      Click here for more details.

      Base Branch:

      Branch Name: master
      Branch GIT ID: 78a6ca4c2979693076ee386c488ca64ed9999dc8

      Sender Branch:

      Branch Name: LPD-68453_FixFilterClear
      Branch GIT ID: 13d91610f462a19ae56d27744f90cc291a047900

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

      @liferay-continuous-integration
      Copy link
      Collaborator

      @liferay-continuous-integration
      Copy link
      Collaborator

      ✔️ ci:test:stable - 11 out of 11 jobs passed

      ❌ ci:test:relevant - 14 out of 16 jobs passed in 1 hour 3 minutes

      Click here for more details.

      Base Branch:

      Branch Name: master
      Branch GIT ID: 78a6ca4c2979693076ee386c488ca64ed9999dc8

      Upstream Comparison:

      Branch GIT ID: 2018e7596b60007f382316cc000ce88d1994c6fa
      Jenkins Build URL: EE Development Acceptance (master) - 195 - 2025-10-16[12:39:48]

      ci:test:stable - 11 out of 11 jobs PASSED
      11 Successful Jobs:
        ci:test:relevant - 14 out of 16 jobs PASSED

        2 Failed Jobs:

        14 Successful Jobs:
          For more details click here.

          Failures unique to this pull:

          For upstream results, click here.

          Test bundle downloads:

          @liferay-continuous-integration
          Copy link
          Collaborator

          @thektan
          Copy link
          Collaborator Author

          thektan commented Oct 18, 2025

          @dsanz @markocikos As a heads up I changed the solution to only return a new object since the config in URL navigation would break (when clearing the filters, navigating back and forward if the filter wasn't mutated 🙃.

          Looks like this allows everything to work as expected now:

          export function deactivateFilter(filter: any) {
          	filter.active = false;
          	filter.odataFilterString = undefined;
          	filter.selectedData = undefined;
          
          	return {
          		...filter,
          	};
          }
          

          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.

          LGTM 👍

          @thektan
          Copy link
          Collaborator Author

          thektan commented Oct 20, 2025

          ci:forward:force

          @liferay-continuous-integration
          Copy link
          Collaborator

          CI is automatically triggering the following test suites:

          •     ci:test:relevant
          •     ci:test:sf
          •     ci:test:stable

          The pull request will automatically be forwarded to the user brianchandotcom if the following test suites complete:

          •     ci:test:relevant
          •     ci:test:sf
            AND If the following test suites pass:
          •     ci:test:stable

          @liferay-continuous-integration
          Copy link
          Collaborator

          Skipping previously completed test suites:

          • ci:test:relevant
          • ci:test:sf
            AND Skipping previously passed test suites:
          • ci:test:stable

          @liferay-continuous-integration
          Copy link
          Collaborator

          Test suite default has been triggered on http://test-1-26

          @liferay-continuous-integration
          Copy link
          Collaborator

          All required test suite(s) completed.
          Forwarding pull request to brianchandotcom.
          Console

          @liferay-continuous-integration
          Copy link
          Collaborator

          Pull request has been successfully forwarded to brianchandotcom#166833
          Console

          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