Add unit test cover + nl2br for location in event emails

[eileenmcnaughton]

Overview

Add unit test cover + nl2br for location in event emails

Before

In looking to swap the notice-y {$location.address.1.display} for the token event.location I realised that the line breaks were not being converted to page breaks

After

The html version of the token now converts line breaks to page breaks & is tested.

Technical Details

This highlights that in fact what is being rendered is

<div class="location vcard"><span class="adr"><span class="street-address">8 Baker Street</span><br />
<span class="extended-address">Upstairs</span><br />
<span class="locality">London</span>,<br />
</span></div>

I want to confirm this is desirable & if so tokens should support it. I found this library https://github.com/jeroendesloovere/vcard

There is a question as to how it would 'look' - we really should support address display at the api-v4 level -

1. what is the name for the pseudofield for address display? 'title'? 'display'? (I thought of title initially because it is what we would want to show for an address 'title' in autocomplete?)
2. if we want to offer a vcard output is that a separate field?

Comments

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

I think it would be great for the API to give formatted output, but keep in mind that everything in a v4 get has to be (mostly) performed within SQL, because it has to work across joins, with grouping, having, etc. Gone are the days where the API always returns a flat array and you can easily tweak the output simply by looping through the key-value pairs.

That said, there is still a per-field formatting callback available, and it does work, as long as all the necessary data was retrieved from the sql query (you can't just assume that the api call had included city, state, country, etc. in the SELECT clause).

So to sum up, yes it can be done, but it should be done the APIv4-way utilizing event callbacks for building the sql and for the post-formatting.

[eileenmcnaughton]

@colemanw ok - makes sense - how would is look in terms of the contract if we add it?

FWIW we COULD do the whole dang thing with crazy complex sql - probably

[eileenmcnaughton]

Note the formats are

1. html vcard format

2. text
   BEGIN:VCARD
   VERSION:4.0
   FN:Simon Perreault
   N:Perreault;Simon;;;ing. jr,M.Sc.
   BDAY:--0203
   GENDER:M
   EMAIL;TYPE=work:simon.perreault@viagenie.ca
   END:VCARD

3. plain text address (ie the html should degrade to plain text in an email - the token processor would handle that but I include it for completeness)

[eileenmcnaughton]

BUT - I first want to question - is it a good thing to put vcard html in emails - I can't decide if it is

a) by design for good reasons
b) by design for bad reasons
c) an unintended consequence of displaying vcards on forms

[seamuslee001]

@agileware-justin do you have any thoughts on the format here?

[eileenmcnaughton]

Just noting that the vcard in emails discussion is not blocking on merging this - perhaps I should move to gitlab & we can merge this?

[agileware-justin]

Thanks for the ping @seamuslee001 - I am intrigued by the vcard discussion and think that's a discussion to be had on gitlab - maybe it will evolve into a PR, maybe not.

I'm not really across what this PR is supposed to do exactly but vcard doesn't look like the primary objective.

[eileenmcnaughton]

Right this PR has the much more modest goal of replacing "\n" line breaks with <br> line breaks in the event location token & adding testing - it was just that it turned out we actually put a vcard html chunk in the emails - which seems odd to me but maybe it's deliberate

[colemanw]

@colemanw ok - makes sense - how would is look in terms of the contract if we add it?

FWIW we COULD do the whole dang thing with crazy complex sql - probably

@eileenmcnaughton we could add an APIv4 pseudo-field called something like "address_formatted" much like our other pseudo-fields we'd create a callback function which would generate the sql for it. I actually think it would be doable to turn this into SQL: just have to translate the api3-style field names, wrap them all in IFNULL conditions and put it all into one big CONCAT().

Except... hang on, that wouldn't work in pure sql because pseudoconstants have to be replaced. Technically that setting also supports custom fields... that might be a real pain to get working because you'd have to fudge the lookup as a subquery or something.

[demeritcowboy]

This has merge conflicts.

[eileenmcnaughton]

@demeritcowboy I fixed the merged conflicts & it has passed tests

[demeritcowboy]

I'm going to merge but noting it means that if you replace the current $location expression in the tpl with {event.location} you get something pretty different.