Openfoodnetwork: Images broken in v3

[luisramos0]

Description

Looks like images are broken in v3 in AUS and UK staging servers. We need to investigate.

Expected Behavior

Product images show up.

Actual Behaviour

No images.

Steps to Reproduce

Go anywhere on staging AUS and check product images.

Workaround

none yet.

Severity

bug-s1: a critical feature is broken: checkout, payments, signup, login

We need to look at this immediately if we want to release this week.

[Matt-Yorkley]

I've been investigating this a bit. It looks like there have been some slight changes to the way assets are precompiled, which affect images added in /assets/images/*.

[sauloperez]

Oh wait, I was just about to start working on it.

[Matt-Yorkley]

It seems to affect some images and not others. Quick example, if you go to a shop page and look at the search bar, it should have this icon in the searchbar:

It's missing in Rails 4.

[Matt-Yorkley]

Static image precompiling issues

I'll write up some other notes here that are important background for the issue, as I think we maybe haven't seen the last of it:

• Rails 4 now only serves assets (including images) from the /public folder.
• During the precompile step Rails 4 compiles assets (including images under /app/assets/images) and moves them to the /public folder, with an added fingerprint. It does not move across non-fingerprinted versions of the images/assets (this is a breaking change).
• In Rails 4, the way images are linked to seems to be a bit different. In order to generate a correct image path, image helpers need to be used in a very specific way, otherwise the fingerprint will not be included, so the page will try to load a non-fingerprinted path, which technically shouldn't exist, but...
• The way we handle some assets in ofn-install is that we symlink a shared assets folder into the build before precompiling (this isn't new, we've been doing it since 2014). The reason we do this is to handle shared assets that are instance-specific (custom banners, logos, PDFs, etc) that are kept on the server and not in the codebase. The (unintended) result of this is that some precompiled assets are left in that folder between builds, which is probably not a good idea. We end up with old precompiled (fingerprinted) assets that are out of date and just taking up space, and additionally there seem to be some non-fingerprinted versions of old assets here (like images) that are present in this folder. This is why we're seeing some of these issues with some images and not others, namely: certain old images are present in non-fingerprinted form, but new ones are not.
• So basically the Rails 4 changes have highlighted an issue that wasn't really visible before, which is that some of our assets and the way we link to them are actually a bit of a mess. We haven't been doing it "correctly", but the shared assets folder meant that the problem was obfuscated and everything seemed to be working ok.
• This should only apply to static images kept in the main repo under /app/assets/images, and not to other more dynamic assets like product images, enterprise images, etc.

Correctly generating an image URL

For reference, paths for images and icons should be written like this:

• some form of image helper should _always_ be used, there are various helpers depending on the context and desired output, such as: image_tag, image_path, etc...
• paths should be relative to the /app/assets/images folder
• if a path to a static image is not passed to a helper, or is given with a / at the beginning, it will _not_ correctly include the fingerprint, and possibly break
• for example an image under /app/assets/images/home/banner.jpg should be: image_path("home/banner.jpg"). This will correctly add the fingerprint when generating the image URL.
• Rails 4 includes some new helpers for use within SASS files as well, check out #image-url

[luisramos0]

awesome, I added this last part to the wiki: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Developer-Guidelines

[sauloperez]

Nice write up @Matt-Yorkley ! That we had a mess with assets across this repo and ofn-install was kind of known. Know we know what is exactly :muscle: .

As per your investigation is there something we need to do to undo the mess in ofn-install? All that seems incredibly brittle and too unknown. We should get to vanilla Rails asset management to avoid trouble.

[Matt-Yorkley]

Yes, I was thinking that too. The way we move instance-specific assets into the build needs to be rethought, for sure. I think we'll need to proceed a bit cautiously though...

[mkllnk]

Great summary @Matt-Yorkley!

The keeping of old assets was very useful for zero-downtime deployments in the past. We could precompile the new assets and then do other stuff like migrations while the site was still working and pointing to the old fingerprints. Only after reloading unicorn pages would link to the new fingerprints.

Now we re-link the app directory and reload unicorn very closely together, possibly within the same second (sometimes slowed down by Ansible connections). So replacing the assets directory would introduce only minimal downtime (broken pages due to JS and CSS not found).

I'm still wondering about the approach of re-writing unicorn.rb during deploy so that we don't need to re-link the directory and reload unicorn in two different steps. Reloading unicorn would then automatically load a new config with a new app directory and we would ensure an atomic switch to the new version. But that's close to micro-optimisation at the moment.