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

[ADD] [web_export_view] export all records if all checkbox is selected #785

Merged
merged 1 commit into from
Nov 8, 2017

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented Oct 31, 2017

this allows to export all records when the all records checkbox is selected.

I wasn't very happy with the implementation choice of fishing out the data from the DOM, that's why I changed this to read from the underlying dataset. Commit 5ca37d4 contains a version that manipulates the DOM to do what we want, but that's slow and very unwieldy. What do @eLBati @simahawk @StefanRijnhart @lepistone (current contributors) think?

@hbrunn hbrunn added this to the 8.0 milestone Oct 31, 2017
@hbrunn hbrunn changed the title [ADD] export all records if all checkbox is selected [ADD] [web_export_view] export all records if all checkbox is selected Oct 31, 2017
@lepistone
Copy link
Member

Hi @hbrunn ! I haven't been involved with this for a while now. I'll let the others speak out. Thanks!

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks! I tested this change succesfully exporting both individual records and all records. I also checked the correct formatting of floats with different decimal separators.

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

Thanks.
Should the same be applied to the more recent versions of odoo?

@hbrunn
Copy link
Member Author

hbrunn commented Nov 6, 2017

if we agree this is a cleaner implementation and if I didn't destroy any feature this way, yes, I think we should (being unsure if this tacitly removes some feature is the reason I harassed all contributors I could find)

@StefanRijnhart
Copy link
Member

I'd merge, but it needs a version bump

@hbrunn
Copy link
Member Author

hbrunn commented Nov 7, 2017

right, thanks. Any reasons for not having a CI check for this? (it's my understanding that actually applying the rules means a version bump for a changed comma)

@hbrunn hbrunn force-pushed the 8.0-web_export_view-active_domain branch from 256eef3 to c71f633 Compare November 7, 2017 07:24
@StefanRijnhart StefanRijnhart merged commit 1492afd into OCA:8.0 Nov 8, 2017
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