Skip to content

Master cv df side panel bis flda matho#8637

Open
fdamhaut wants to merge 10 commits into
masterfrom
master-cv-df-side-panel-bis-flda-matho
Open

Master cv df side panel bis flda matho#8637
fdamhaut wants to merge 10 commits into
masterfrom
master-cv-df-side-panel-bis-flda-matho

Conversation

@fdamhaut
Copy link
Copy Markdown
Contributor

Description:

description of this task, what is implemented and why it is implemented that way.

Task: 4072640

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 (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Copy Markdown
Collaborator

robodoo commented May 11, 2026

Pull request status dashboard

fdamhaut added 4 commits May 12, 2026 09:30
Make the criterion input sheet dependent, this mean that when saved,
their content will always show the value based on their sheetId

Task: 4072640
@fdamhaut fdamhaut force-pushed the master-cv-df-side-panel-bis-flda-matho branch from 29d5c11 to ff4f263 Compare May 12, 2026 08:01
const stringRanges = ranges.map((range) => this.getters.getRangeString(range, cmd.sheetId));
if (stringRanges.some((xc) => !this.getters.isRangeValid(xc))) {
return CommandResult.InvalidRange;
if (ranges.some((range) => range.sheetId !== cmd.sheetId)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why don't we have to check here anymore if the range is still valid ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed 😄

this.closeSidePanel();
}

closeSidePanel() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of the name. From my understanding, it will change the switch to the "detailed" side panel of a CF, not really close a side panel. Or maybe the name should suggest that if closes and open another ?

Copy link
Copy Markdown
Contributor Author

@fdamhaut fdamhaut May 12, 2026

Choose a reason for hiding this comment

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

From my understanding, it will change the switch to the "detailed" side panel of a CF, not really close a side panel.

It's the opposite, we are leaving the detailed view to go back to the "list" view.
But as it's unclear I will switch to smth more explicit :D

@@ -1,14 +1,18 @@
import { Component, ComponentConstructor, useState } from "@odoo/owl";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo in the commit : The use is allowed to witch sheet (missing the "S" of SWITCH)

get dispatchPayload(): Omit<AddDataValidationCommand, "type"> {
get dispatchPayload(): Omit<AddDataValidationCommand, "type">[] | undefined {
const rule = { ...this.state.rule, ranges: undefined };
let errors = [] as CancelledReason[];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const errors: CancelledReason[] = [];

}
return this.env.model.getters.getRangeDataFromXc(this.props.sheetId, xc);
});
if (errors.length) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is no need to split the case here as if errors.length is falsy, then errors == [], so you can do directly this.state.errors = errors

const { sheetName } = splitReference(xc);
const sheetId = this.env.model.getters.getSheetIdByName(sheetName);
if (sheetName && !sheetId) {
errors = [CommandResult.InvalidRange];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it wanted that if you have a sheetName and no sheetId, you will still call the this.env.model.getters.getRangeDataFromXc ? Shouldn't we return undefined instead, as we already know that we will have an InvalidRange ?

fdamhaut and others added 6 commits May 13, 2026 15:22
Adapt data_validation to criterion changes ans improve the multi-sheet
navigation.

Task: 4072640
Adapt cf to criterion changes and improve the multi sheet navigation.

Task: 4072640
If a new rule has the same criterion and the same blocking behavior as an existing rule,
the new rule is merged with the existing one.

Task: 4072640
Allow selection input to force the sheet prefix to be shown.

Task: 4072640
…e sheets

Before this commit:
The user is allowed to switch sheet and select a range in a different sheet
but then the DV is applied to this range in the active sheet while naming the other sheet in the description

After this commit:
If the user selects ranges on different sheets, we create one DV rule per sheet
with the same criterion and values but with the correct ranges for each sheet.

Task: 4072640
If the user selects ranges on different sheets, we create one CF per sheet
with the same criterion and values but with the correct ranges for each sheet.

Task: 4072640
@fdamhaut fdamhaut force-pushed the master-cv-df-side-panel-bis-flda-matho branch from d0a92d0 to 5a98e4b Compare May 13, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants