Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Export fixes #182

Open
wants to merge 3 commits into
base: 1.0.0-wip
Choose a base branch
from
Open

Export fixes #182

wants to merge 3 commits into from

Conversation

mwgamble
Copy link

  • Fix issue with exporting more than 1000 items in a grid at one time. If you attempted to export more than one page worth of results for a grid whose collection modifies the select object to add new column correlations in the _beforeLoad() method, you would get an error because it would attempt to create the correlation on every page iteration of the export process.
  • Fix issue with exporting any number of items. You would always get one more than you asked for.
  • Fix general issues with Transactions grid due to it not resetting the collection object when preparing the collection for a 2nd time.

@mage-eag
Copy link
Collaborator

Thanks for the fixes to the transactions grids !

Regarding the number of exported items, I get the opposite behaviour : the exported count is correct when using if (++$exported > $total), and I get one less than expected when using >= instead. Can you check on your side if you get one too many result due to the value of $total being altered at some point, or due to some specific use cases ?

@mwgamble
Copy link
Author

mwgamble commented Aug 3, 2015

My apologies, there was indeed a bug in our code. We had inadvertently moved the call_user_func_array call above the if statement in question. The rest of the PR is still necessary to view and export items in the transactions grid, so I'll update it soon.

Matthew Gamble added 3 commits November 16, 2015 13:05
Clone a collection's select object when cloning the collection
2b89bcd 
PHP object cloning is shallow. Therefore, any objects owned by the
cloned object are also owned by the original object, because it's all
passed around by reference. This means that if the select object is
modified after the clone, this will still affect the original
collection's select object. This creates problems when you have
collections that modify the select object in the _beforeLoad() method.

We encountered issues when exporting the Sales Transactions grid,
where it would complain that it had already added a column name
correlation whenever it reached the 2nd page when exporting a
collection. This was happening because it was using the same shared
select object from the original collection.

To get around this, we now clone the original select object, and
manually set it on the cloned collection on each iteration of the loop,
using reflection to do so. This appears to work fine for all grids, and
is a general improvement.

It is worth noting that this exact issue is probably why Magento never
made exporting possible on some grids, such as the Sales Transactions
grid.
Fix for transactions grid collection not being reloaded properly
1ac0401 
The extension relies on the fact that a new collection object is created
during the second call to _prepareCollection(). This doesn't happen with
the transactions grid, because it always first looks to see if there is
a collection already set. It does this because there is a subclass which
also creates a collection, and utilises this class to do most of the
work.

We don't need this for the main transactions grid, so we unset the
collection before calling _prepareCollection() for the 2nd time. This
makes everything work a lot more nicely.
Bump rewrite code version to reflect new changes
4812ef5
@mwgamble
Copy link
Author

Hi, I just wanted to let you know that I've rebased this branch on top of the latest commits from the 1.0.0-wip branch to resolve the merge conflicts that were there. You should be able to safely merge it in now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mwgamble @mage-eag