search

OCA / spreadsheet

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

[ADD] spreadsheet_oca & spreadsheet_dashboard_oca #2

Closed etobella closed 1 year ago

etobella commented 1 year ago

This PR contains two modules:

TODO:

https://user-images.githubusercontent.com/28590170/210117043-65740b18-a368-41b6-bc03-1cde568af555.mp4

@pedrobaeza

etobella commented 1 year ago

Saving on dashboards now works. About the create, I forgot about it. At the begining I tried to do it with a Form JS Class, but at the end I had to find another way. I need to modify tree view. I will do ASAP :wink:

legalsylvain commented 1 year ago

Thanks for your answer ! Please ping me when you want review. Happy new years !

etobella commented 1 year ago

Tree modified. Creation is allowed properly :tada:

etobella commented 1 year ago

Just one thing. Should we use dms functionality in order to avoid to redo folders and permissions?

Next step would be to improve views on DMS. But it might be achieved :smile:

pedrobaeza commented 1 year ago

Yeah, I was thinking in a spreadsheet_dms_oca module. For this one, I think we should set a basic security without folders, having groups per spreadsheet. Forcing to use DMS for a basic security seems too much for me.

etobella commented 1 year ago

Hi @legalsylvain

Just a few things:

Multi edit video :smile:

https://user-images.githubusercontent.com/28590170/210182863-3a2f98fa-cc87-449b-8dbb-434b5976fe52.mp4

legalsylvain commented 1 year ago

Menu names, fixed

The last instance on runboat have the same "spreadsheet spreasheet" title.

About your questions, I am not sure what are you asking. Do you mean to access another spreadsheet? Might be possible, but don't know how. Using the same logic from ODOO.BALANCE. It might be possible.

I mean : Go to "Dashboard" > "Configuration" > Click on a group (like "Sale") and here, have the possibility to "Add a line".

image

Does it makes sense ?

IMO the main option should be to use the spreadsheet and adding a button to access the form on the spreadsheet view

As you wish. I guess, that having a "classic" tree / form view structure and just add a button "Open" in the tree view (and in the form view) could be the more simple to implement and the UI the most "awaited". (I mean, all the rest of odoo works the same, when you click on a line of a tree view, the form view is opened). Anyway, implementing a form view is a must have, because otherwise, no "duplicate" function is available. Not a blocking point, thought.

Multi edit

1) Tested in spreadsheet spreadsheet entry. Work like a charm ! Nice ! Wow effect !

2) Regression : go to Dashboard / configuration / open a sheet : You have an error :

AttributeError: type object 'spreadsheet.dashboard' has no attribute 'send_spreadsheet_message'

I guess you should implement send_spreadsheet_message in this model too.

Side questions :

etobella commented 1 year ago

I mean : Go to "Dashboard" > "Configuration" > Click on a group (like "Sale") and here, have the possibility to "Add a line". Does it makes sense ? Yes! Fixed :smile:

IMO the main option should be to use the spreadsheet and adding a button to access the form on the spreadsheet view

As you wish. I guess, that having a "classic" tree / form view structure and just add a button "Open" in the tree view (and in the form view) could be the more simple to implement and the UI the most "awaited". (I mean, all the rest of odoo works the same, when you click on a line of a tree view, the form view is opened). Anyway, implementing a form view is a must have, because otherwise, no "duplicate" function is available. Not a blocking point, thought.

After a second thought, I think you are right. Changes applied :smile:

Multi edit

  1. Tested in spreadsheet spreadsheet entry. Work like a charm ! Nice ! Wow effect !
  2. Regression : go to Dashboard / configuration / open a sheet : You have an error :

AttributeError: type object 'spreadsheet.dashboard' has no attribute 'send_spreadsheet_message'

I guess you should implement send_spreadsheet_message in this model too.

I passed the logic to an abstract in order to avoid the same error on the future :wink:

Side questions :

  • I see that it's possible to have spreadsheets with no name. (when creating a new one, by default). Maybe having a default "Untitled" or "No name" could be more appropriate.

:thinking: Maybe we need to make it required

legalsylvain commented 1 year ago

Thanks for all the changes. Nice refactoring with the abstract btw !

etobella commented 1 year ago

We can now export from Pivot Tables :tada:

https://user-images.githubusercontent.com/28590170/210276196-e6ec4978-425d-4bc5-8a86-c5f9edaeb2f4.mp4

chienandalu commented 1 year ago

Bravo!

pedrobaeza commented 1 year ago

Please add also this icon, and put it on the menu (it can directly be the SVG I think).

icon

etobella commented 1 year ago

Hi @pedrobaeza Your inline comments have been attended

About your main comment :smile:

  • The dashboard edition doesn't work:

True, should be fixed now.

  • On this matter, I prefer that the dashboards with XML-ID can't be overwritten, and only allows to save a copy.

Well, I understand your concern, but I am not sure if it is the right approach :thinking: Also, the system checks who can edit it, and it is only editable by the right users, so only administrator users are allowed to edit spreadsheet dashboards.

  • I also misses the possibility to save a spreadsheet as a dashboard.

The idea is to create it directly there on the dashboard configuration.

  • About security, I would create 2 groups:

Spreadsheets / User Spreadsheets / Manager You may want that your users don't access spreadsheets at all, so not putting any of them, the main menu is not shown, as well as the button on the pivot views. If you put user, then you will be able to see those spreadsheets that you are owner, contributor or reader, and edit the first 2. Being manager, you see and edit all.

It makes sense, I will do it ASAP

  • From the pivot view, is there any possibility of adding it to an existing spreadsheet? It would be a very nice feature.

It is already done :smile:

  • Spreadsheets can be saved with no name, so please put it as required.

Yes, ASAP :wink:

Maybe it's better to split the pull request in 2 parts and let this one for the base module and handle the dashboard thing in another.

I will close this one when it is ready in order to keep the history in this PR. then I will create two new ones with only one commit

Also, thanks for the icon, it is already here :smile:

pedrobaeza commented 1 year ago

OK, tested. The Edit button still appears when you don't have any Spreadsheet group, but then it appears an error when you click it. Maybe it's better to hide such button. About having that edit button on standard dashboards, it seems it's still usable for spreadsheet / user.

You can now proceed to clean the commit history and let only the main module here.

etobella commented 1 year ago

Done!