matthiask / plata

Plata - the lean and mean Django-based Shop
https://plata-django-shop.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
202 stars 63 forks source link

Deleting orders fails when stock app isn't included in INSTALLED_APPS #27

Closed arteme closed 11 years ago

arteme commented 12 years ago

Hi,

If I have the settings as:

INSTALLED_APPS = (
    ...
    'plata.discount',
    'plata.payment',
    'plata.product',
    'plata.shop',
    ...

Then deleting orders fails:

In [1]: Order.objects.all().delete()
---------------------------------------------------------------------------
DatabaseError                             Traceback (most recent call last)
<ipython-input-1-2dbc0623de24> in <module>()
----> 1 Order.objects.all().delete()

/.../env/local/lib/python2.7/site-packages/django/db/models/query.pyc in delete(self)
    508 
    509         collector = Collector(using=del_query.db)
--> 510         collector.collect(del_query)
    511         collector.delete()
    512 

/.../env/local/lib/python2.7/site-packages/django/db/models/deletion.pyc in collect(self, objs, source, nullable, collect_related, source_attr, reverse_dependency)
    175                     if not sub_objs:
    176                         continue
--> 177                     field.rel.on_delete(self, field, sub_objs, self.using)
    178 
    179             # TODO This entire block is only needed as a special case to

/.../env/local/lib/python2.7/site-packages/django/db/models/deletion.pyc in CASCADE(collector, field, sub_objs, using)
     15 def CASCADE(collector, field, sub_objs, using):
     16     collector.collect(sub_objs, source=field.rel.to,
---> 17                       source_attr=field.name, nullable=field.null)
     18     if field.null and not connections[using].features.can_defer_constraint_checks:
     19         collector.add_field_update(field, None, sub_objs)

/.../env/local/lib/python2.7/site-packages/django/db/models/deletion.pyc in collect(self, objs, source, nullable, collect_related, source_attr, reverse_dependency)
    173                 else:
    174                     sub_objs = self.related_objects(related, new_objs)
--> 175                     if not sub_objs:
    176                         continue
    177                     field.rel.on_delete(self, field, sub_objs, self.using)

/.../env/local/lib/python2.7/site-packages/django/db/models/query.pyc in __nonzero__(self)
    128             return bool(self._result_cache)
    129         try:
--> 130             iter(self).next()
    131         except StopIteration:
    132             return False

/.../env/local/lib/python2.7/site-packages/django/db/models/query.pyc in _result_iter(self)
    116                 raise StopIteration
    117             if len(self._result_cache) <= pos:
--> 118                 self._fill_cache()
    119 
    120     def __nonzero__(self):

/.../env/local/lib/python2.7/site-packages/django/db/models/query.pyc in _fill_cache(self, num)
    873             try:
    874                 for i in range(num or ITER_CHUNK_SIZE):
--> 875                     self._result_cache.append(self._iter.next())
    876             except StopIteration:
    877                 self._iter = None

/.../env/local/lib/python2.7/site-packages/django/db/models/query.pyc in iterator(self)
    289             klass_info = get_klass_info(model, max_depth=max_depth,
    290                                         requested=requested, only_load=only_load)
--> 291         for row in compiler.results_iter():
    292             if fill_cache:
    293                 obj, _ = get_cached_row(row, index_start, db, klass_info,

/.../env/local/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in results_iter(self)
    761         if self.query.select_for_update and transaction.is_managed(self.using):
    762             transaction.set_dirty(self.using)
--> 763         for rows in self.execute_sql(MULTI):
    764             for row in rows:
    765                 if resolve_columns:

/.../env/local/lib/python2.7/site-packages/django/db/models/sql/compiler.pyc in execute_sql(self, result_type)
    816 
    817         cursor = self.connection.cursor()
--> 818         cursor.execute(sql, params)
    819 
    820         if not result_type:

/.../env/local/lib/python2.7/site-packages/django/db/backends/util.pyc in execute(self, sql, params)
     38         start = time()
     39         try:
---> 40             return self.cursor.execute(sql, params)
     41         finally:
     42             stop = time()

/.../env/local/lib/python2.7/site-packages/django/db/backends/sqlite3/base.pyc in execute(self, query, params)
    335         query = self.convert_query(query)
    336         try:
--> 337             return Database.Cursor.execute(self, query, params)
    338         except Database.IntegrityError, e:
    339             raise utils.IntegrityError, utils.IntegrityError(*tuple(e)), sys.exc_info()[2]

DatabaseError: no such table: stock_stocktransaction

If I add "plata.product.stock" to INSTALLED_APPS then deletion succeeds.

andialbrecht commented 12 years ago

What's the solution for this issue? Issue #25 mentions this issue in a somewhat different context, but is there a way to not use "plata.product.stock"?

andialbrecht commented 12 years ago

oh, sorry, I've hit the submit button a bit too early....

To me it seems that the ForeignKey from StockTransaction to OrderPayment is the culprit. When Django's doing it's magic when deleting objects it encounters StockTransaction somehow regardless of the PLATA_STOCK_TRACKING setting.

matthiask commented 12 years ago

Yes, I think that's correct. The stock models should be skipped if plata.product.stock is NOT added to INSTALLED_APPS. I'm not sure why this isn't happening.

andialbrecht commented 12 years ago

@matthiask is there a workaround? What side-effects would it have to add plata.product.stock to INSTALLED_APPS? Do I have to manage stock items myself then?

matthiask commented 12 years ago

Andi,

It should not have any side effects beneath the addition of two database tables. As long as PLATA_STOCK_TRACKING is False (which is the default) you should not have to change anything, especially not manage stock items yourself.

I'd still like to know why you are seeing this particular problem. I hope I'll be able to reproduce this issue.

Thanks, Matthias

andialbrecht commented 12 years ago

Thanks for the feedback! I'll try it with plata.shop.stock in INSTALLED_APPS.

The problem is a bit weird to reproduce. It occurs in some of our testing instances from time to time, but I was not able to reproduce it on a development machine or even to repeat the behavior on a testing machine after it occurred. But I keep an eye on it and hope to come up with some more information when this happens, especially how Django comes to the conclusion, that it has to look at the stock model at some point.

andialbrecht commented 12 years ago

FTR, when enabling stock this way, the same problem with migrations as addressed in issue #26 comes up.

matthiask commented 11 years ago

I've been finally hit by this issue too. Luckily, I'm able to reproduce it in the development server too...

matthiask commented 11 years ago

This is probably a bug in Django: http://code.djangoproject.com/ticket/19422

matthiask commented 11 years ago

The only place I found where the stock models are imported if PLATA_STOCK_TRACKING=False was in the reporting module.

The bug can be triggered by importing plata.product.stock.models early somewhere in a models file which is referenced through INSTALLED_APPS.

I'm not sure whether there is an easy way to fix the problem without patching Django first. Maybe make the period and stock transaction model abstract?

tomchristie commented 11 years ago

Note: Same issue on a difference project... https://github.com/tomchristie/django-rest-framework/issues/705

matthiask commented 11 years ago

Workaround for later reference:

https://github.com/tomchristie/django-rest-framework/commit/c5b98f0d106758298edf045e7bb44ecd7e4b9629