Skip to content
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.

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

[FW][FIX] filters: filter can hide all the visible rows of a sheet #2199

Closed
wants to merge 1 commit into from

Conversation

fw-bot
Copy link
Collaborator

@fw-bot fw-bot commented Mar 9, 2023

Description

It's currently possible to use data filters to hide all the rows in a sheet,
leaving a buggy sheet with no visible rows.

Fixing this is a bit tricky, since the filtered rows are an UI concept, and
thus we cannot rely on the allowDispatch of "HIDE_COLUMNS_ROWS" to prevent
the user from hiding all the rows. Using allowDispatch isn't possible
because the filtered values (and the values of the cells) are different
from an user to another. This means that the allowDispatch could return
a success for an user, and a failure for the other, leading to a
dis-synchronized model state for the 2 user.

Fortunately we can fix this by disabling data filters whose header row is
hidden (by the user or by another filter). By construction of the filters,
it becomes impossible to hide all the rows of a sheet.

This may still becomes an issue in the future if we implement something
like filters being applied to a column header. In that case, we would need
to handle sheets with all their rows hidden.

Odoo task ID : 3205608

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_lt("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

Forward-Port-Of: #2148

It's currently possible to use data filters to hide all the rows in a sheet,
leaving a buggy sheet with no visible rows.

Fixing this is a bit tricky, since the filtered rows are an UI concept, and
thus we cannot rely on the allowDispatch of "HIDE_COLUMNS_ROWS" to prevent
the user from hiding all the rows. Using allowDispatch isn't possible
because the filtered values (and the values of the cells) are different
from an user to another. This means that the allowDispatch could return
a success for an user, and a failure for the other, leading to a
dis-synchronized model state for the 2 user.

Fortunately we can fix this by disabling data filters whose header row is
hidden (by the user or by another filter). By construction of the filters,
it becomes impossible to hide all the rows of a sheet.

This may still becomes an issue in the future if we implement something
like filters being applied to a column header. In that case, we would need
to handle sheets with all their rows hidden.

Odoo task 3205608

X-original-commit: 1eee732
@robodoo
Copy link
Collaborator

robodoo commented Mar 9, 2023

@fw-bot
Copy link
Collaborator Author

fw-bot commented Mar 9, 2023

@hokolomopo @rrahir this PR targets master and is the last of the forward-port chain containing:

To merge the full chain, say

@fw-bot r+

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@rrahir
Copy link
Collaborator

rrahir commented Mar 9, 2023

@fw-bot r+

robodoo pushed a commit that referenced this pull request Mar 9, 2023
It's currently possible to use data filters to hide all the rows in a sheet,
leaving a buggy sheet with no visible rows.

Fixing this is a bit tricky, since the filtered rows are an UI concept, and
thus we cannot rely on the allowDispatch of "HIDE_COLUMNS_ROWS" to prevent
the user from hiding all the rows. Using allowDispatch isn't possible
because the filtered values (and the values of the cells) are different
from an user to another. This means that the allowDispatch could return
a success for an user, and a failure for the other, leading to a
dis-synchronized model state for the 2 user.

Fortunately we can fix this by disabling data filters whose header row is
hidden (by the user or by another filter). By construction of the filters,
it becomes impossible to hide all the rows of a sheet.

This may still becomes an issue in the future if we implement something
like filters being applied to a column header. In that case, we would need
to handle sheets with all their rows hidden.

Odoo task 3205608

closes #2199

X-original-commit: 1eee732
Signed-off-by: Rémi Rahir (rar) <[email protected]>
@robodoo robodoo temporarily deployed to merge March 9, 2023 10:06 Inactive
@robodoo robodoo closed this Mar 9, 2023
@robodoo robodoo added the 16.2 label Mar 9, 2023
@fw-bot fw-bot deleted the master-16.0-filter-all-rows-adrm-NLT8-fw branch March 23, 2023 10:47
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.

4 participants