609098 - Can't save documents

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task, P1)

Description

[James Socol [:jsocol, :james]]

I tried to edit a document (change tags, or title, or Fx versions) and I always get the following stack trace. The document:

* is in en-US,
* has no parent, and
* is_localizable == True.

This may or may not be a dupe of bug 608100, or fixed at the same time, but I think it describes a different case.

Stack trace below:

-----------------------

Environment:

Request Method: POST
Request URL: http://sumo/en-US/kb/Cookies/edit
Django Version: 1.2.3
Python Version: 2.6.2
Installed Applications:
['django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.messages',
 'django.contrib.admin',
 'tower',
 'jingo_minify',
 'kitsune',
 'authority',
 'access',
 'sumo',
 'search',
 'forums',
 'djcelery',
 'cronjobs',
 'notifications',
 'identicons',
 'questions',
 'kadmin',
 'taggit',
 'flagit',
 'upload',
 'product_details',
 'wiki',
 'kbforums',
 'dashboards',
 'gallery',
 'customercare',
 'twitter',
 'chat',
 'django_extensions',
 'django_nose',
 'test_utils',
 'debug_toolbar']
Installed Middleware:
('multidb.middleware.PinningRouterMiddleware',
 'django.middleware.transaction.TransactionMiddleware',
 'sumo.middleware.LocaleURLMiddleware',
 'sumo.middleware.Forbidden403Middleware',
 'django.middleware.common.CommonMiddleware',
 'sumo.middleware.NoCacheHttpsMiddleware',
 'commonware.middleware.NoVarySessionMiddleware',
 'commonware.middleware.FrameOptionsHeader',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'sumo.anonymous.AnonymousIdentityMiddleware',
 'sumo.middleware.TikiCookieMiddleware',
 'twitter.middleware.SessionMiddleware',
 'sumo.middleware.PlusToSpaceMiddleware')


Traceback:
File "/var/www/kitsune/vendor/src/django/django/core/handlers/base.py" in get_response
  100.                     response = callback(request, *callback_args, **callback_kwargs)
File "/var/www/kitsune/vendor/src/django/django/views/decorators/http.py" in inner
  37.             return func(request, *args, **kwargs)
File "/var/www/kitsune/vendor/src/django/django/contrib/auth/decorators.py" in _wrapped_view
  25.                 return view_func(request, *args, **kwargs)
File "/var/www/kitsune/apps/wiki/views.py" in edit_document
  214.                     doc = doc_form.save(request.locale, None)
File "/var/www/kitsune/apps/wiki/forms.py" in save
  97.         doc.save()
File "/var/www/kitsune/apps/wiki/models.py" in save
  234.         self._validate_is_localizable()
File "/var/www/kitsune/apps/wiki/models.py" in _validate_is_localizable
  195.                                   unicode(self), unicode(self.parent)))

Exception Type: ValidationError at /kb/Cookies/edit
Exception Value:

Comment 1

[Erik Rose [:erik][:erikrose]]

Probably the document was migrated with an invalid combination of parent and is_localizable set, and save() just caught it. Either we should have migrations not create invalid data (my hope), or we'll need to lift these restrictions. If you actually created this doc by hand, please reopen, because that shouldn't be able to happen.

Comment 2

[Erik Rose [:erik][:erikrose]]

Nm, I missed the initial data at the top of this bug. Those constitute a valid state. But it looks like something else is going on--perhaps something unexpected in a view.

Comment 3

[James Socol [:jsocol, :james]]

Something is setting is_localizable to False in the doc_form instance, which is causing the exception to be correctly raised. Maybe just the form is rendering something wrong?

Comment 4

[James Socol [:jsocol, :james]]

Looks like we've figured this out:

When a document with existing translations is edited, it doesn't include the is_localizable checkbox (because you can't mark it non-localizable after translations exist). However, because it's not included, the form in the view treats it as False. Either we need to set it to true in the view when translations exist, or we need to include it as a hidden input. (We could also include the checkbox, checked, and disabled, but we still need the hidden input since disabled fields aren't submitted with the form.)

Comment 5

[Erik Rose [:erik][:erikrose]]

*runs screaming*

Comment 6

[James Socol [:jsocol, :james]]

Ricky's patch[1] fixes the original issue, but has brought to light another one.

It seems that something in the many-to-one relationship between Documents and FirefoxVersions (presumably OperatingSystems, too) is not getting saved/cleared correctly. I get an IntegrityError with the following steps:

1) Create a document that applies to any FirefoxVersion.
2) Resave the document part of the form. (Changes optional, but don't _remove_ any FirefoxVersions.)
3) PROFIT! I mean IntegrityError!

[1] https://github.com/rlr/kitsune/commit/ac223a988ed79

=============================

Environment:

Request Method: POST
Request URL: http://sumo/en-US/kb/Cookies/edit?opendescription=1
Django Version: 1.2.3
Python Version: 2.6.2
Installed Applications:
// ...snip...


Traceback:
File "/var/www/kitsune/vendor/src/django/django/core/handlers/base.py" in get_response
  100.                     response = callback(request, *callback_args, **callback_kwargs)
File "/var/www/kitsune/vendor/src/django/django/views/decorators/http.py" in inner
  37.             return func(request, *args, **kwargs)
File "/var/www/kitsune/vendor/src/django/django/contrib/auth/decorators.py" in _wrapped_view
  25.                 return view_func(request, *args, **kwargs)
File "/var/www/kitsune/apps/wiki/views.py" in edit_document
  214.                     doc = doc_form.save(request.locale, None)
File "/var/www/kitsune/apps/wiki/forms.py" in save
  111.         doc.firefox_versions = ffv
File "/var/www/kitsune/apps/wiki/models.py" in __setattr__
  278.         super(Document, self).__setattr__(name, value)
File "/var/www/kitsune/apps/wiki/models.py" in <lambda>
  104.                                 setattr(self, direct_attr, val))
File "/var/www/kitsune/apps/wiki/models.py" in __setattr__
  278.         super(Document, self).__setattr__(name, value)
File "/var/www/kitsune/vendor/src/django/django/db/models/fields/related.py" in __set__
  389.         manager.add(*value)
File "/var/www/kitsune/vendor/src/django/django/db/models/fields/related.py" in add
  417.                     obj.save()
File "/var/www/kitsune/vendor/src/django/django/db/models/base.py" in save
  434.         self.save_base(using=using, force_insert=force_insert, force_update=force_update)
File "/var/www/kitsune/vendor/src/django/django/db/models/base.py" in save_base
  527.                     result = manager._insert(values, return_id=update_pk, using=using)
File "/var/www/kitsune/vendor/src/django/django/db/models/manager.py" in _insert
  195.         return insert_query(self.model, values, **kwargs)
File "/var/www/kitsune/vendor/src/django/django/db/models/query.py" in insert_query
  1479.     return query.get_compiler(using=using).execute_sql(return_id)
File "/var/www/kitsune/vendor/src/django/django/db/models/sql/compiler.py" in execute_sql
  783.         cursor = super(SQLInsertCompiler, self).execute_sql(None)
File "/var/www/kitsune/vendor/src/django/django/db/models/sql/compiler.py" in execute_sql
  727.         cursor.execute(sql, params)
File "/var/www/kitsune/vendor/src/django/django/db/backends/util.py" in execute
  15.             return self.cursor.execute(sql, params)
File "/var/www/kitsune/vendor/src/django/django/db/backends/mysql/base.py" in execute
  86.             return self.cursor.execute(query, args)
File "/usr/lib/python2.6/site-packages/MySQLdb/cursors.py" in execute
  173.             self.errorhandler(self, exc, value)
File "/usr/lib/python2.6/site-packages/MySQLdb/connections.py" in defaulterrorhandler
  36.     raise errorclass, errorvalue

Exception Type: IntegrityError at /kb/Cookies/edit
Exception Value: (1062, "Duplicate entry '1-629' for key 'item_id'")

Comment 7

[Paul Craciunoiu [:paulc]]

(In reply to comment #6)
> Ricky's patch[1] fixes the original issue, but has brought to light another
> one.
> 
> It seems that something in the many-to-one relationship between Documents and
> FirefoxVersions (presumably OperatingSystems, too) is not getting saved/cleared
> correctly. I get an IntegrityError with the following steps:
It's possible we need to do a [fxvers/oses]_set.delete() before saving. Seems like this issue was introduced by the fix in bug 605275.

Comment 8

[Paul Craciunoiu [:paulc]]

I'll fix it...

Comment 9

[Paul Craciunoiu [:paulc]]

Should be able to save metadata now...
https://github.com/jsocol/kitsune/commit/d5751fbd0ab

Comment 10

[Rebecca Billings [:rbillings]]

Verified fixed