Merge lp:~arthru/prestashoperpconnect/fix-1199664 into lp:prestashoperpconnect

[arthru]

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hi,

I fear that it was merged a bit too quick.

Say you have products 1-2500, then 4000-4500, with a hole between the 2 ranges.

If you search from 0 to 1000, then 1001 to 2000, etc. and stops until nothing is found, you will miss the last range of products.

I know that's a very hypothetical issue but it could be prevented as of today.
I don't know too much the Prestashop's API but one way would be to ask the greater ID and loop until reached, is it possible? Otherwise, maybe the last call could be an unlimited query (> last imported) but you could run in the same issue...

review: Needs Fixing (code review, no test)

[arthru (arthru)]

I don't see the point with the two ranges.

limit=0,1000 will get 1000 elements from the first, and we loop until the response is not composed of 1000 elements.

The limit parameter does not filter for the id less than a number, it is really the 1000 first elements, disregarding if their id is less or more than 1000.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Ok, so that's only wrong assumption from my part, and my dumb ignorance of the Prestashop's API.
Nice to have such an option in the API!

So, LGTM.

review: Approve (code review, no test)

[arthru (arthru)]

About the merge too quick, I just did a wrong move with bazaar, my bad... (I thought Sébastien merged the branch... but it was just me and my mistake...).

Sorry...

=== modified file 'prestashoperpconnect/unit/import_synchronizer.py'
--- prestashoperpconnect/unit/import_synchronizer.py	2013-07-07 09:46:53 +0000
+++ prestashoperpconnect/unit/import_synchronizer.py	2013-07-10 09:49:24 +0000
@@ -156,12 +156,28 @@
     items to import, then it can either import them directly or delay
     the import of each item separately.
     """
+    page_size = 1000
 
     def run(self, filters=None):
         """ Run the synchronization """
+        if filters is None:
+            filters = {}
+        if 'limit' in filters:
+            self._run_page(filters)
+            return
+        page_number = 0
+        filters['limit'] = '%d,%d' % (page_number * self.page_size, self.page_size)
+        record_ids = self._run_page(filters)
+        while len(record_ids) == self.page_size:
+            page_number += 1
+            filters['limit'] = '%d,%d' % (page_number * self.page_size, self.page_size)
+            record_ids = self._run_page(filters)
+
+    def _run_page(self, filters):
         record_ids = self.backend_adapter.search(filters)
         for record_id in record_ids:
             self._import_record(record_id)
+        return record_ids
 
     def _import_record(self, record):
         """ Import a record directly or delay the import of the record """