Comments

JoshaHubbers created an issue. See original summary.

joshahubbers’s picture

StatusFileSize
new1.66 KB

This patch can be applied AFTER the patch in #3053594: Add correspondentieAdres and verblijfsOfCorrespondentieadres tokens is applied.
The prefill is only changed for "vestiging" because the "persoon" prefill is already widely used, and we don't dare to change that without consulting users.

ralphvdhoudt’s picture

Title: Remove html break from address token » Add new tokens with values without <br/>
Issue summary: View changes
Status: Needs review » Needs work
ralphvdhoudt’s picture

Status: Needs work » Needs review
StatusFileSize
new18.79 KB

Created a patch with single line tokens and refactored the tokens to have subgroups for verblijfsadres, correspondentieAdres

ralphvdhoudt’s picture

Issue summary: View changes
tvoesenek’s picture

Status: Needs review » Needs work

Patch #4 looks good, the grouping gives makes selecting the right token easier. But if you now select one of these parent tokens:

[dvg_personen:verblijfsadres]
[dvg_personen:verblijfsOfCorrespondentieadres]
[dvg_personen:correspondentieAdres]

No data is rendered and you get the following notice:

Notice: Undefined property: stdClass::$_ in _dvg_stuf_bg_tokens_replace_natuurlijk_persoon() (regel 416 van /profiles/dvg/modules/contrib/dvg_stuf_bg/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module).
ralphvdhoudt’s picture

StatusFileSize
new19.93 KB

Only works after applying https://www.drupal.org/files/issues/2019-05-22/add-sub.correspondentieAd...

[dvg_personen:verblijfsadres]
[dvg_personen:verblijfsOfCorrespondentieadres]
[dvg_personen:correspondentieAdres]

These tokens fallback to the naamAdresWoonplaats token

Also made some changes to the tokens because the wrong ones were used

['tokens']['correspondentieAdres']['wpl.woonplaatsNaam']
['tokens']['verblijfsOfCorrespondentieadres']['wpl.woonplaatsNaam']

Ans made some changes to _dvg_stuf_bg_tokens_webform_address, to use the correct fields

ralphvdhoudt’s picture

Status: Needs work » Needs review
paulvandenburg’s picture

Status: Needs review » Needs work

I've found some code standard/typo issues.

  1. +++ b/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module
    @@ -9,6 +9,150 @@
    +  // Subtokens verblijsadres etc.
    

    Typo in verblijfsadres

  2. +++ b/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module
    @@ -300,7 +304,15 @@ function dvg_stuf_bg_tokens_tokens($type, $tokens, array $data = array(), array
    +  // fallback.
    

    Please add a more descriptive comment.

  3. +++ b/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module
    @@ -418,7 +448,13 @@ function _dvg_stuf_bg_tokens_replace_natuurlijk_persoon($name, $original, $repla
    +  // fallback.
    

    Similar comment

  4. +++ b/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module
    @@ -436,11 +472,24 @@ function _dvg_stuf_bg_tokens_replace_vestiging($name, $original, $replacements,
    +            $value .= _dvg_stuf_bg_tokens_webform_address($prefill, 'verblijfsadres');
    

    Don't specify an optional parameter equal to the default value.

  5. +++ b/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module
    @@ -436,11 +472,24 @@ function _dvg_stuf_bg_tokens_replace_vestiging($name, $original, $replacements,
    +            $value = _dvg_stuf_bg_tokens_webform_address($prefill, 'verblijfsadres');
    

    Same here

  6. +++ b/modules/dvg_stuf_bg_tokens/dvg_stuf_bg_tokens.module
    @@ -470,12 +519,13 @@ function _dvg_stuf_bg_tokens_replace_vestiging($name, $original, $replacements,
    +function _dvg_stuf_bg_tokens_webform_address($prefill, $address_type = 'verblijfsadres', $seperator = '<br/>') {
    

    Typo, should be separator

ralphvdhoudt’s picture

Status: Needs work » Needs review
StatusFileSize
new20.02 KB
new3.6 KB

Resolved feedback #9

paulvandenburg’s picture

StatusFileSize
new1.62 KB
new21.27 KB

Looks good.
However I did get a few notices, which are not really related to this issue perhaps but fixing them here is faster than opening a new ticket.
See attached.

tvoesenek’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new21.17 KB

Reroll of #11 against the latest dev. The fixed notices also look good to me.

paulvandenburg’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.