OCA / spreadsheet

GNU Affero General Public License v3.0
46 stars 47 forks source link

Get Odoo Client Error when deleting a worksheet #32

Closed chrisandrewmann closed 5 months ago

chrisandrewmann commented 5 months ago

Module

spreadsheet_oca

Describe the bug

Get Odoo Client Error when deleting a sheet. Details of the error are as follows: UncaughtPromiseError > TypeError Uncaught Promise > env.askConfirmation is not a function

TypeError: env.askConfirmation is not a function at Proxy.action (http://192.168.56.55:8169/web/assets/432-b2d3786/spreadsheet.o_spreadsheet.min.js:949:197) at Menu.activateMenu (http://192.168.56.55:8169/web/assets/432-b2d3786/spreadsheet.o_spreadsheet.min.js:697:60) at Menu.onClickMenu (http://192.168.56.55:8169/web/assets/432-b2d3786/spreadsheet.o_spreadsheet.min.js:713:11) at Menu.hdlr5 (eval at compile (http://192.168.56.55:8169/web/assets/425-76b1f63/web.assets_common.min.js:2051:370), :50:27) at Object.mainEventHandler (http://192.168.56.55:8169/web/assets/425-76b1f63/web.assets_common.min.js:2096:77) at HTMLDivElement.listener (http://192.168.56.55:8169/web/assets/425-76b1f63/web.assets_common.min.js:1292:15)

To Reproduce

Steps to reproduce the behavior:

  1. Go to a new spreadsheet or existing
  2. Right click a sheet and delete

Expected behavior Should be able to delete sheets without error

Previously reported in this closed issue: https://github.com/OCA/spreadsheet/issues/17

legalsylvain commented 5 months ago

Hi @chrisandrewmann. http://oca-spreadsheet-16-0-6bf589a8c659.runboat.odoo-community.org sorry I can't reproduce the bug on runboat. could you be more precise ? Thanks !

chrisandrewmann commented 5 months ago

Hi @legalsylvain sorry if not clear, see the attached GIF.

Steps are:

spreadsheet-delete-error

legalsylvain commented 5 months ago

Hi @chrisandrewmann.

thanks for your extra explanation. Bug reproduced on runboat. For the next time, please provide log error in debug mode. The text is more explicite :

TypeError: env.askConfirmation is not a function
    action@http://URL/web/assets/debug/spreadsheet.o_spreadsheet.js:5998:30 (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:5993)
    activateMenu@http://URL/web/assets/debug/spreadsheet.o_spreadsheet.js:4276:39 (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:4271)
    onClickMenu@http://URL/web/assets/debug/spreadsheet.o_spreadsheet.js:4356:26 (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:4351)
    slot1/hdlr5<@http://URL/web/assets/debug/web.assets_common.js line 21935 > Function:50:27 (/web/static/lib/owl/owl.js:5532)
    mainEventHandler@http://URL/web/assets/debug/web.assets_common.js:22222:25 (/web/static/lib/owl/owl.js:5819)
    listener@http://URL/web/assets/debug/web.assets_common.js:16760:20 (/web/static/lib/owl/owl.js:357)

investigation :

/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:                env.askConfirmation(_lt("We found data next to your selection. Since this data was not selected, it will not be sorted. Do you want to extend your selection?"), () => {
/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:        action: (env) => env.askConfirmation(_lt("Are you sure you want to delete this sheet ?"), () => {
/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:            env.askConfirmation(AddMergeInteractiveContent.MergeIsDestructive, () => {

I guess that the function is define somewhere in Odoo EE.

Solution:

Redevelop the function in the OCA module. Could you make a PR ?

CC : @etobella

chrisandrewmann commented 5 months ago

@legalsylvain and @etobella I have a working mockup but not time to do a PR yet.

Modified the spreadsheet_renderer.esm.js with following changes:

        useSubEnv({
            saveSpreadsheet: this.onSpreadsheetSaved.bind(this),
            editText: this.editText.bind(this),
            askConfirmation: this.askConfirmation.bind(this),
        });

    askConfirmation(content, confirm) {
        this.state.dialogContent = content;
        this.state.dialogDisplayed = true;
        this.state.hideInputBox = true;
        this.confirmDialog = () => {
            confirm();
            this.closeDialog();
        };
    }
etobella commented 5 months ago

It seems ok. Can you create a PR?

chrisandrewmann commented 5 months ago

It seems ok. Can you create a PR?

Will do ASAP. I didn't like the fact the input box was still editable even with an alert-only message, so made some further amendments so it can display only text and make the dialog multi-purpose. Not done a lot with OWL yet so still learning, but liking it!

etobella commented 5 months ago

OWL and JS is weird at the begining, but at the end, you will enjoy it

chrisandrewmann commented 5 months ago

PR done @etobella https://github.com/OCA/spreadsheet/pull/33