Skip to content

[IMP] spreadsheet_oca: Allow to import to existent spreadsheets#5

Merged
OCA-git-bot merged 5 commits into
OCA:16.0from
tegin:16.0-imp
Mar 13, 2023
Merged

[IMP] spreadsheet_oca: Allow to import to existent spreadsheets#5
OCA-git-bot merged 5 commits into
OCA:16.0from
tegin:16.0-imp

Conversation

@etobella
Copy link
Copy Markdown
Member

Affects to both modules

@pedrobaeza

@etobella
Copy link
Copy Markdown
Member Author

Peek 2023-01-20 22-58

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jan 21, 2023
@pedrobaeza
Copy link
Copy Markdown
Member

Thanks for this feature, Enric!

Testing on runboat, I found that adding to an existing spreadsheet, it replaces the content that existed on the first cell, so resulting that you can't add 2 pivots through this technique without any in-between manipulation.

Is it possible to insert the pivot table in the first empty row?

@etobella
Copy link
Copy Markdown
Member Author

Well, it is added on a new sheet, so I don't see the problem 🤔

We can try to add to the last line, but I think it might be better to use a new sheet in order to avoid errors

@jiivii252

This comment was marked as spam.

@pedrobaeza
Copy link
Copy Markdown
Member

@etobella thanks for the extra features, that are also very important.

The question I comment is still important, as this replaces anything you have, being a previous pivot, graph, text, or whatever, so you adding to an existing spreadsheet should add such content on the first empty row.

@pedrobaeza
Copy link
Copy Markdown
Member

Ey, what I'm seeing now is that it's put in a new sheet instead. That's a bit confusing...

@etobella
Copy link
Copy Markdown
Member Author

etobella commented Feb 1, 2023

Well, we can try to add it to the end, but it will be at the end of the first sheet, as we are unable to know which is the right one... It mitght be possible (not sure) to find the sheets and allow to add it where we want by the user, but it is confusing IMO.

At the end, I think Odoo is doing a similar approach 🤔

@pedrobaeza
Copy link
Copy Markdown
Member

Isn't there "active sheet"? If so, we can add it at the end of such active one.

@etobella
Copy link
Copy Markdown
Member Author

etobella commented Feb 2, 2023

But, If two users accessed the view, they might have different active sheets. In this case, we should need to create a list of users/sheets in order to fix that. However, if another user removes the last sheet you opened, that might be a problem 😭

@pedrobaeza
Copy link
Copy Markdown
Member

OK, I think a good compromise is to add it always at the end of the first sheet. The question is not to force people to move the content from the newly created sheet to the one they want (which will be 99% of the times the first one).

@etobella etobella force-pushed the 16.0-imp branch 2 times, most recently from 89cc825 to d6be331 Compare February 12, 2023 19:53
@etobella
Copy link
Copy Markdown
Member Author

After reviewing the options, I am able to add it at the end of the sheet, but I found an issue. On sheet creation, the system adds 100 rows, so it will be added at row 100 on small sheets, this can be confusing 😭 I may try to move the view there, but then, we will need to remove rows. WDYT?

The other option, would be to check line by line from the last one, but that can be an extra cost :sad:

@pedrobaeza
Copy link
Copy Markdown
Member

As the operation is only done on specific moments, I don't think checking the content of the cells reverse from down to top until the first with content is found is too much. Have you tried to see the times?

@etobella
Copy link
Copy Markdown
Member Author

@pedrobaeza I was able to do something for pivot tables. It does not work properly for graphs

Copy link
Copy Markdown
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK, I think it's enough for now, as the graph doesn't erase previous data. It simply gets over the rest and you can drag and drop it.

Thanks for the efforts!

/ocabot merge minor

@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-5-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit af63883 into OCA:16.0 Mar 13, 2023
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at e34dcd4. Thanks a lot for contributing to OCA. ❤️

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