715563 - Automatically generate banner image filenames

Status: VERIFIED FIXED

(Firefox Affiliates Graveyard :: affiliates.mozilla.org, defect)

Description

[Osmose [:osmose, :mkelly]]

When uploading a banner image, the site should rename the banner to some standard filename that will always be the same for that banner image. This will allow us to update a banner image and change the image on every site that it is used.

Currently, the filename is not changed, which means that uploading a new image will not update currently existing banners.

Comment 1

[Anthony Ricaud (:rik)]

Any suggestion/recommandation about the format that should be used for the filename?

I'd say {banner_id}_{size}_{color}_{locale}.jpg

Comment 2

[Osmose [:osmose, :mkelly]]

Sounds good to me.

Comment 3

[Chelsea Novak [:chelsea]]

+1

Comment 4

[Anthony Ricaud (:rik)]

Pull request in https://github.com/mozilla/affiliates/pull/80

Comment 5

[Anthony Ricaud (:rik)]

I've updated the pull request.

Comment 6

[[github robot]]

Commit pushed to develop at https://github.com/mozilla/affiliates

https://github.com/mozilla/affiliates/commit/9141d2f50f0183da80276d6e6677cdfe947e530c
Generate banner image filenames

This adds a new FileSystemStorage to overwrite existing files.
fix bug 715563

Comment 7

[Matt Brandt [:mbrandt]]

I'm seeing this type of traceback coming in:

Traceback (most recent call last):

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/core/handlers/base.py", line 111, in get_response
   response = callback(request, *callback_args, **callback_kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/contrib/admin/options.py", line 307, in wrapper
   return self.admin_site.admin_view(view)(*args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/utils/decorators.py", line 93, in _wrapped_view
   response = view_func(request, *args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/views/decorators/cache.py", line 79, in _wrapped_view_func
   response = view_func(request, *args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/contrib/admin/sites.py", line 197, in inner
   return view(request, *args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/utils/decorators.py", line 28, in _wrapper
   return bound_func(*args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/utils/decorators.py", line 93, in _wrapped_view
   response = view_func(request, *args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/utils/decorators.py", line 24, in bound_func
   return func(self, *args2, **kwargs2)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/db/transaction.py", line 217, in inner
   res = func(*args, **kwargs)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/contrib/admin/options.py", line 985, in change_view
   self.save_formset(request, form, formset, change=True)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/contrib/admin/options.py", line 677, in save_formset
   formset.save()

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/forms/models.py", line 482, in save
   return self.save_existing_objects(commit) + self.save_new_objects(commit)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/forms/models.py", line 599, in save_existing_objects
   saved_instances.append(self.save_existing(form, obj, commit=commit))

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/forms/models.py", line 470, in save_existing
   return form.save(commit=commit)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/forms/models.py", line 363, in save
   fail_message, commit, construct=False)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/forms/models.py", line 85, in save_instance
   instance.save()

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/db/models/base.py", line 460, in save
   self.save_base(using=using, force_insert=force_insert, force_update=force_update)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/db/models/base.py", line 525, in save_base
   values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/db/models/fields/files.py", line 255, in pre_save
   file.save(file.name, file, save=False)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/db/models/fields/files.py", line 92, in save
   self.name = self.storage.save(name, content)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/core/files/storage.py", line 49, in save
   name = self._save(name, content)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/apps/shared/storage.py", line 47, in _save
   file_move_safe(tmp_path, full_path)

 File "/data/www/affiliates-dev.allizom.org/affiliates-app/vendor/src/django/django/core/files/move.py", line 66, in file_move_safe
   (not allow_overwrite and os.O_EXCL or 0))

OSError: [Errno 17] File exists: '/data/www/affiliates-dev.allizom.org/affiliates-app/media/uploads/banners/c5e52102ecc5985cdb2f4012408846c3496ff892.png'

Comment 8

[Osmose [:osmose, :mkelly]]

This appears to be fixed on dev now with the fixes in https://github.com/mozilla/affiliates/commit/15316a5da3ff6e209202e297d67e0968501c7e95 and https://github.com/mozilla/affiliates/commit/915942a576d04c5f46929da23bffd682bf78d29a

Comment 9

[Osmose [:osmose, :mkelly]]

It's important to note that we've changed the format of the filenames to be a hash; the hash is generated from the banner image's parent banner id, width, height, color, and locale. 

The important rule is that if you replace a banner image with another one of the same width and height, and change nothing else about the banner image, the hash should stay the same (allowing us to make adjustments to the image and have it affect all current affiliate links.

Comment 10

[Rajeev N B [:rshetty]]

QA Verified.
On Banner image upload the banner is provided a unique hash value . The hash value remains the same when the banner is replaced with some another banner of same height and width thus helping to update a banner image and change the image on every site that it is used.