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

[17.0][MIG] spreadsheet_oca #59

Open
wants to merge 128 commits into
base: 17.0
Choose a base branch
from
Open

Conversation

etobella
Copy link
Member

Finishes migration of #40

etobella and others added 30 commits February 16, 2025 10:53
With this change we are adding an extra layer of security. Without it, any user was able to sniff how all happened using something like:

    const any_spreadsheet_id = 1234;
    const channel = "spreadsheet_oca;spreadsheet.spreadsheet;" + any_spreadsheet_id;
    bus_service.addChannel(channel);
    bus_service.addEventListener("spreadsheet_oca", (message) => /* every revision arrives here */

With the change, we verify the access to the model with a similar logic to `web_editor` fields
OCA-git-bot and others added 16 commits February 16, 2025 11:10
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: spreadsheet-16.0/spreadsheet-16.0-spreadsheet_oca
Translate-URL: https://translation.odoo-community.org/projects/spreadsheet-16-0/spreadsheet-16-0-spreadsheet_oca/
Currently translated at 100.0% (101 of 101 strings)

Translation: spreadsheet-16.0/spreadsheet-16.0-spreadsheet_oca
Translate-URL: https://translation.odoo-community.org/projects/spreadsheet-16-0/spreadsheet-16-0-spreadsheet_oca/it/
Before these changes, when a table or chart was added with an active
user filter, the user's selections were not respected, either in the
groupings or in the measures.

Thanks to these changes, we remove the conflicting keys from the context
passed through the searchParams, thus respecting the user's selection in
the table/chart itself.

Steps to reproduce the problem:

1. Go to a pivot or graph
2. Save a filter
3. Modify the groups or measures
4. Add to a spreadsheet

Current behavior: The table will not be added with the current selection
but it will be added with the filters groups and measures.

Expected behavior:  The table will be added with the current selection.
Currently translated at 100.0% (101 of 101 strings)

Translation: spreadsheet-16.0/spreadsheet-16.0-spreadsheet_oca
Translate-URL: https://translation.odoo-community.org/projects/spreadsheet-16-0/spreadsheet-16-0-spreadsheet_oca/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: spreadsheet-16.0/spreadsheet-16.0-spreadsheet_oca
Translate-URL: https://translation.odoo-community.org/projects/spreadsheet-16-0/spreadsheet-16-0-spreadsheet_oca/
Currently translated at 100.0% (102 of 102 strings)

Translation: spreadsheet-16.0/spreadsheet-16.0-spreadsheet_oca
Translate-URL: https://translation.odoo-community.org/projects/spreadsheet-16-0/spreadsheet-16-0-spreadsheet_oca/it/
@legalsylvain
Copy link
Contributor

Hi @etobella. Thanks for porting this module.
could you take a time to review this V16 pending PR #57.
So the last improvments could go in the last version.

thanks !

@chrisandrewmann
Copy link
Contributor

chrisandrewmann commented Feb 16, 2025

Thanks @etobella.
I've done a quick test and all looks good, apart from a couple of localisation issues noticed (i'm based in the UK so using GBP currency).

In spreadsheet_oca on Community V16 (and V17 Enterprise), you are able go to "Format > Number > Custom currency", to set a currency other than USD.
This menu is missing currently, looking at the code it seems to be related to setting the locale?
image

On Enterprise you do it by going to File > Settings and changing the locale:
image

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Thanks @etobella! Can you execute pre-commit?

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

When I tried to add a dynamic table from a pivot view I'm getting next error:

OwlError: The following error occurred in onWillStart: "row.map is not a function"
    Error
        at wrapError (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:10180:23) (/web/static/lib/owl/owl.js:2631)
        at onWillStart (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:10224:29) (/web/static/lib/owl/owl.js:2675)
        at SpreadsheetRenderer.setup (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:69959:5) (/spreadsheet_oca/static/src/spreadsheet/bundle/spreadsheet_renderer.esm.js:89)
        at new ComponentNode (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:9928:28) (/web/static/lib/owl/owl.js:2379)
        at http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:13524:28 (/web/static/lib/owl/owl.js:5975)
        at ActionSpreadsheetOca.template (eval at compile (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:13248:20), <anonymous>:16:16) (/web/static/lib/owl/owl.js:5699)
        at Fiber._render (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:9278:38) (/web/static/lib/owl/owl.js:1729)
        at Fiber.render (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:9270:18) (/web/static/lib/owl/owl.js:1721)
        at ComponentNode.initiateRender (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/web.assets_web.js:9950:23) (/web/static/lib/owl/owl.js:2401)

Caused by: TypeError: row.map is not a function
    at http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:67186:24 (/spreadsheet/static/src/pivot/pivot_table.js:85)
    at Array.map (<anonymous>)
    at new SpreadsheetPivotTable (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:67182:27) (/spreadsheet/static/src/pivot/pivot_table.js:81)
    at PivotCorePlugin.handle (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:67828:31) (/spreadsheet/static/src/pivot/plugins/pivot_core_plugin.js:91)
    at Model.dispatchToHandlers (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:57848:25) (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:57731)
    at http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:57792:30 (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:57675)
    at StateObserver.recordChanges (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:55841:13) (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:55724)
    at Model.dispatch (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:57788:62) (/spreadsheet/static/src/o_spreadsheet/o_spreadsheet.js:57671)
    at ActionSpreadsheetOca.importDataPivot (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:70316:23) (/spreadsheet_oca/static/src/spreadsheet/bundle/spreadsheet_action.esm.js:250)
    at async ActionSpreadsheetOca.importData (http://oca-spreadsheet-17-0-pr59-1b4cf23a9dc5.runboat.odoo-community.org/web/assets/debug/spreadsheet.o_spreadsheet.js:70333:7) (/spreadsheet_oca/static/src/spreadsheet/bundle/spreadsheet_action.esm.js:267)

I have set next configuration to get the error:

image

@chrisandrewmann
Copy link
Contributor

chrisandrewmann commented Feb 18, 2025

Ref #59 (comment)

I've got a working POC which fixes this. How do you want me to provide it? @etobella
In summary:

  • Adds the [Settings] menu item to the file menu, allowing user to change the locale
  • Sets the default currency format for new sheets based on the current company
  • Retrieves all locales and currencies
  • Enables the Format > Number > Custom Currency sidebar to select currencies
  • In addition i've been working on adding the ability to delete pivots and lists from the sheet.

@etobella
Copy link
Member Author

Make a PR an I can review it

@chrisandrewmann
Copy link
Contributor

chrisandrewmann commented Feb 18, 2025

Make a PR an I can review it

@etobella PR is here: dixmit#1

@etobella
Copy link
Member Author

@CarlosRoca13 The error is related to some functionality that was added to limit the rows and columns. I fixed it for rows, but the same patch is not working for columns. I will investigate why 😢

Set default currency for new sheets.
Fix crash on insertDynamicPivot.
Fix typos (noficiation > notification).
Add feature to delete pivots and lists from sheet via button on sidebars.
@etobella etobella force-pushed the 17.0-mig-spreadsheet branch from 198dfad to 8ace503 Compare February 23, 2025 17:33
Copy link
Contributor

@chrisandrewmann chrisandrewmann left a comment

Choose a reason for hiding this comment

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

Crashes when adding new chart to sheet

get menuProps() {
const menu = this.env.model.getters.getChartOdooMenu(this.props.figureId);
var result = {
record: this.record,
Copy link
Contributor

Choose a reason for hiding this comment

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

Causes crash when adding a new chart to sheet.
Replace with fieldString: _t("Menu Items"),

import {Many2XAutocomplete} from "@web/views/fields/relational_utils";
import {patch} from "@web/core/utils/patch";
import {useService} from "@web/core/utils/hooks";

Copy link
Contributor

Choose a reason for hiding this comment

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

Add import {_t} from "@web/core/l10n/translation";

import {Domain} from "@web/core/domain";
import {Many2XAutocomplete} from "@web/views/fields/relational_utils";
import {useService} from "@web/core/utils/hooks";

Copy link
Contributor

Choose a reason for hiding this comment

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

Add import {_t} from "@web/core/l10n/translation";

get menuProps() {
const menu = this.env.model.getters.getChartOdooMenu(this.props.figureId);
var result = {
record: this.record,
Copy link
Contributor

Choose a reason for hiding this comment

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

Causes crash when adding a new chart to sheet.
Replace with fieldString: _t("Menu Items"),

class SpreadsheetDownloadXLSX(Controller):
@route("/spreadsheet/xlsx", type="http", auth="user", methods=["POST"])
def download_spreadsheet_xlsx(self, zip_name, files, **kw):
files = json.loads(files)
Copy link
Contributor

Choose a reason for hiding this comment

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

Crashes when exporting to XLSX when filestorage object not JSON.

Add this line above:
if hasattr(files, "read"): files = files.read().decode("utf-8")

Copy link
Contributor

@chrisandrewmann chrisandrewmann left a comment

Choose a reason for hiding this comment

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

Downloading XLSX crashes, small amendment to download_spreadsheet_xlsx

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.