String default for FileBrowserField not working

[carlosgeos]

Clone of sehmaschine#358, I decided to open it here since it looks more actively maintained.

A string default for FileBrowserField does not work since it looks for .path on a string, at least on PostgreSQL:

django-filebrowser-no-grappelli/filebrowser/fields.py

Lines 113 to 116 in 62c01cb

	def get_prep_value(self, value):
	if not value:
	return value
	return value.path

Of course this fails with:

AttributeError: 'str' object has no attribute 'path'

Other examples can be found in the original issue linked.

I'll submit the same PR, but maybe there is a better solution to this problem.

[smacker]

Hi @carlosgeos ! Thank you for the bug report!

Could you please help me reproduce the error?

I tried using a clean state with Django 2.0 & 2.1 and didn't get the error. My steps:

1. checkout this repo & cd to it
2. To ensure environment is correct run tests ./runtests_in_env.sh. (It runs tests on different versions of django. You can change DJANGO_VERSIONS and keep only 2.1 to speedup things)
3. add default value to manual test generator:

--- a/manual_test_2.sh
+++ b/manual_test_2.sh
@@ -76,7 +76,7 @@ from filebrowser.fields import FileBrowseField, FileBrowseUploadField

 class DemoItem(models.Model):
     title = models.CharField('Title', max_length=210)
-    attach = FileBrowseField('Attach', max_length=200)
+    attach = FileBrowseField('Attach', max_length=200, default='uploads/myfile.jpg')
     upload = FileBrowseUploadField('Attach', max_length=200)
 " > dummy/models.py

4. Generate test project: ./manual_test_2.sh 2.1
5. Activate virtualenv: source envs/2.1/bin/activate
6. Go to project: cd projects/fb-dj2.1/
7. Change admin password: python manage.py changepassword admin
8. Put any image to the path ./media/uploads/myfile.jpg
9. Run server: python manage.py runserver 0.0.0.0:8011
10. Go to http://127.0.0.1:8011/admin/dummy/demoitem/add/ it is displayed correctly with image preview. Save button also works.

Let me know what should I change to reproduce the issue or you can try my steps yourself.

[carlosgeos]

Yes that is correct, there is no problem when using sqlite. Also, if only applying one migration, it works on all databases. (this error happens on subsequent migrations)

To reproduce, add a PostgreSQL database to the settings (I set up a remote one you can use):

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
    },
    'other': {
        'ENGINE': 'django.db.backends.postgresql',
        'NAME': 'default',
        'USER': 'myuser',
        'PASSWORD': 'mypass',
        'HOST': '172.104.224.150',
        'PORT': '5432',
    }
}

and change models.py to reflect some change in this default string for a FileBrowseField. For example, uploads/myfile.jpg to uploads/myfile_change.jpg

makemigrations and migrate with:

python manage.py migrate --database=other

When it tries to apply the newly created migration (with the change), it will fail with:

filebrowser/fields.py", line 116, in get_prep_value
    return value.path
AttributeError: 'str' object has no attribute 'path'

---

To reproduce with this repo's testing setup, I did:

1. Add the psycopg2-binary==2.7.7 dependency to tests/requirements.txt
2. Add the Postgres db to the settings.py echo area in ./manual_test_2.sh
3. Run ./run_tests_in_env.sh.
4. Run ./manual_test_2.sh 2.1.
5. Get in the virtual env and get in the project.
6. Run python manage.py migrate --database=other. Everything works fine.
7. Change or add the default string for a FileBrowseField
8. Make migrations and migrate to the Postgres DB
9. ERROR with:

  File "/Users/carlos/Documents/django-filebrowser-no-grappelli/projects/fb-dj2.1/filebrowser/fields.py", line 116, in get_prep_value
    return value.path
AttributeError: 'str' object has no attribute 'path'

I don't know why it doesn't fail with SQLite. Also, maybe the PR submitted needs a couple more checks not just at get_prep_value (haven't tested yet).

But the interesting fact is that if I wipe the database and apply only ONE migration containing the info for FileBrowseField, everything works. This makes sense I think since the code checks if it already has a value: if it doesn't, it returns value (and it WORKS) and if it already does, it returns value.path (and it DOESNT WORK with str). As soon as another migration appears that modifies some FileBrowseField, it fails.

[smacker]

Thank you for such detail bug report! I really appreciate it!

The bug is fixed the similar way you proposed but with support to all python versions, tests and FileBrowseUploadField.

I'll publish new version on pypi later today/in the next few days.

[carlosgeos]

Thanks !!

[smacker]

released: https://pypi.org/project/django-filebrowser-no-grappelli/

[carlosgeos]

Quick question: is it useful to include the six package here (README) ?

Because if the package six is not available, the following message appears:

filebrowser/fields.py", line 4, in <module>
    from six import string_types
ImportError: No module named 'six'

[smacker]

I'm sorry, it's my fault. There was an unused import of six in test requirements so I missed that we don't depend on it. I have just released v3.7.7 that removes the new dependency.

Please try new version.