Update descriptions

Created on 6 May 2024, 2 months ago
Updated 8 June 2024, about 1 month ago

Please only use Merge requests for contributing to this issue. Using Gitlab β†’ Please do not use patches for contributing to this project.

Problem/Motivation

Add event descriptions.

Steps to reproduce

N/A

Proposed resolution

N/A

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

πŸ“Œ Task
Status

Fixed

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States nicxvan

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @nicxvan
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • Merge request !10Add descriptions β†’ (Merged) created by nicxvan
  • Issue was unassigned.
  • Status changed to Needs review 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • Status changed to Needs work 2 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thanks @nicxvan for the fast turnaround on this. Two things though:

    • Not sure if we should use translatables here, in the rest of the eca-verse we've used plain english strings only.
    • The description doesn't need to mention, for which events each token is available, that will be discovered by implemented Reflection routines. It should describe the meaning of the token to the end-user, i.e. what it is and maybe what it can be used for.
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Is there a reason not to make them translatable? I had trouble finding documentation on attributes of Attributes.

    Can you give me an example of other tokens you've set up? When I searched a couple I didn't see examples.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Is there a reason not to make them translatable?

    We don't yet have a strategy for translation and will have to introduce that later, when we do so. Having a markup object instead of a string may not hurt, actually. I'm just not sure, since those strings are used in automatic processes for multi-purpose output and I haven't checked, if all channels can cope with markup.

    Can you give me an example of other tokens you've set up?

    In ECA 2 almost all event plugins in sub-modules have token attributes, e.g. \Drupal\eca_config\Plugin\ECA\Event\ConfigEvent

  • Status changed to Needs review 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Thanks for the example!

    I updated the descriptions.

    I also changed the two tokens for commerce_store_* to commerce_tax_* since they are tax objects

    and I split profile into two since one of them is an array of profiles.

  • Status changed to Needs work 2 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thank you @nicxvan, this is looking good but I left a few comments in the MR that we need to still sort out.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I'll take a look!

    I think for the store changes we either provide an update hook, or just post a notice.

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I addressed your comments, but I have one question about replacing configuration, I'd like to rename them to be more informative and accurate.

  • Status changed to RTBC about 2 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thank you @nicxvan for addressing all the topics in the MR, I've marked all threads as resolved now.

    The question is about the renaming of the tokens and how to update existing models, I've done that in an MR of ECA just yesterday. The code is untested at this point, but you can have a look here: https://git.drupalcode.org/project/eca/-/merge_requests/423/diffs#031db4...

    However, the likelihood that anyone used the renamed tokens in eca_commerce is very little, bearing in mind that there are 120 installations, most of which probably still in the testing phase. So, maybe a note in the release notes might be sufficient?

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I'm going to merge this, but just so I don't forget the release notes:

    Renamed: commerce_store_plugin to commerce_tax_plugin
    Renamed: commerce_store_zones to commerce_tax_zones
    Renamed: profile to customer_profile
    Split: profile to profiles

  • Status changed to Fixed about 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States nicxvan
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Hi @nicxvan as we're providing a helper function in ECA 2.0.x-dev, I've now added a post update hook which automatically updates existing ECA models for the changed token names.

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

Production build 0.69.0 2024