Skip to content

Resolve "Fix missing path for avatars of bots"

Manoj M J requested to merge 231251-error-500-in-service-desk-due-missing-asset into master

What does this MR do?

For #231251 (closed), which is a regression caused by the introduction of !36269 (merged)

More details are available in the thread: #231251 (comment 384151812)

Details:

  1. The images currently reside in app/assets folder which is precompiled.
  2. After pre-compilation, which puts all assets in the public/ folder, we remove the entire app/assets folder from disk (discussion on backend channel - internal only): This behaviour is now documented in !37695 (merged)
  3. Because of this, our current code is unable to find the original files that resides inside app/assets, as this directory does not exist anymore.
  4. But it is not necessary to put these images under app/assets/images since we do not need to precompile these avatar images at all - we never serve these images directly to users. We just need the contents of these images to tell Carrierwave to use these as source images for the User avatar.
  5. Because of this, it is fine to move these images to a folder that is not pre-compiled.
  6. lib/assets is an already existing folder in our repo, and this felt like the ideal candidate to move these images to, which is the crux of this MR. This folder is not considered for pre-compilation, so the source remains on disk.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Manoj M J

Merge request reports