CRM-20326 Contact images url storage in the database

Sounds a good improvement to me as far as i understand it. Agree in general that full urls need to be removed wherever they are being used

Sounds good.

My concerns are:
1. Will this cause confusion for users doing an upgrade, where half are in one format, and half in another? Writing a MySQL condition to update is quicker if not having to do an if clause. Or would this enhancement also replace them upon performing the upgrade?
2. Testing it on upgrades for all 3 platforms.

JohnFF

I just have a nasty feeling there was a change from relative urls to full urls for some reason - might be worth checking some git history

John Kirk as part of that change an update script would be written that migrates existing entries in the database to the new format

Eileen McNaughton good suggestion, my gut feeling is that this was made when images stopped being accessed directly but had to be accessed through a proxy script checking access permissions - so people added the full URL for the proxy script. But definitely worth checking ...

Would you lean towards solution 1 (ie. just the file name, same as for the civicrm_file table), or solution 2 (using the [cms.root] token to build the url)? I personally like solution 2 better as it keeps the spirit of the field (image_url), but could easily be steered towards any other solutions.

Nicolas GanivetI feel like Views may have been involved - but perhaps that could be said about anything!

This issue was vaguely familiar to me too.

Here's a ticket where I outlined several problems, including the absolute url one:

https://issues.civicrm.org/jira/browse/CRM-14646

The conclusion was: we should remove the absolute URL, but only in a major upgrade for fear of breaking any external scripts that lookup the field value directly to display the image. And... that change does not seem to have ever been submitted!

Jamie McClelland thanks for the pointer. From the comments in your ticket I gather that the url needs to stay a URL as webform_civicrm generates a pointer to the public Drupal file space (and I imagine we could also use this field to point to a Twitter / Gravatar or public profile picture).

So this eliminates solution 1 proposed above, but solution 2 still seems an improvement as will resolve both problematic use cases (url change and multisite). It seems we could easily do pattern matching for [cms.root] on saving that field, and pattern replacement on extracting it from the database, either in the BAO or on in the API.

Any strong support for that?

My gut reaction was suspicion because [cms.root]/civicrm/contact/imagefile?photo=image.png assumes clean-url notation (incompatible with Joomla/WordPress). To my aesthetic, it would be preferrable for to patch Civi\Core\Paths to support some additional notations like any of these:

• Civi::paths()->getUrl('[civicrm.contactImage?]photo=image.png')
• Civi::paths()->getUrl('route://civicrm/contact/imagefile?photo=image.png') (this feels most likely)
• Civi::paths()->getUrl('[route:civicrm/contact/imagefile]?photo=image.png')

In which case you might use an algorithm like https://gist.github.com/totten/7e37a0e0b2400196b70c20712d1986fa to translate from concrete URLs to abstract URLs.

However, on second look, Nicolas's original suggestion using only [cms.root] may actually be OK – it doesn't matter what the example is that we used, it only matters that the filters (substituting-in and substituting-out the cms.root) are complementary. More specifically, it's OK if

• You don't migrate CMS's
• You don't migrate between subfolders
• You don't toggle the clean-URL. (Either keep it consistently off or consistently on.)
• You intend for image_URL's to be on the same domain as the main web site. (Intentionally serving data within the same page-view from multiple domains could get dicey.)

The original description is about contact image files, but the handling of these and image files stored as custom fields should be consistent. Presumably changing how this is stored would require updates to at least the Views integration, webform_civicrm and civicrm_entity.

For the first of the issues described (changing website url), maybe a script to update the absolute URL's introduces fewer complications?

important fix , and using relative URLs make more sense to me instead of [cms.root] but there could be probably some edge cases I am not aware off .

If anyone's considering putting in place a more wide ranging system of storage of images in Civi, please note this issue: https://issues.civicrm.org/jira/browse/CRM-14537.

This isn't important specifically for contact images in most cases, but the concept of "hang on, is this image we're storing identical to one already in the DB? If so, use that" is a smart idea.

At a high-level these are my leanings:

1. We should avoid storing absolute addresses in the database.
2. There are too many file storage mechanisms in CiviCRM. I haven't wrestled with any of this code lately, but I seem to recall different mechanisms for premiums, contact images... and something else? ... maybe custom file fields?

If the solution to this problem results in the deprecation of a storage mechanism that used in only one place, I think we come out ahead.

+1 to Tim's idea of providing a programming interface for fetching files rather than using [cms.root].

Frank Gómez Check out my comment before yours for a similar example that would be useful for.

One client that I work with imports contact data that includes external URLs for images (a list of MPs). I've not tested whether this works in 4.7.x, but certainly used to in 4.6.x and was extremely helpful. In that use case storage of absolute URLs is of course essential.