WeMoveEU / uk.co.compucorp.civicrm.mailjet

MailJet Extension for CiviCRM
GNU General Public License v2.0
3 stars 5 forks source link

Hard Coded Activity-ID for bounces #16

Open pbatroff opened 4 years ago

pbatroff commented 4 years ago

In Endpoint.php the Activity Id is hard coded to 58 at the moment.

I assumed it is a default CiviCRM Activity, but in a fresh install it is not available. Is this a mistake on my end, or did it change recently and got removed? (Testing on 5.19.4).

Otherwise I would propose to either configure an activity-id, or let the Extension create on on installation and use that instead. (saving the ID to an extension setting).

tttp commented 4 years ago

Indeed, 58 is the hardcoded bounce activity (it always puzzled me that civimail by default doesn't keep a clear and visible history or why an email was put on hold)

Indeed, that would be a great improvement to create the activity type if not existing. My only concern is that it's an hot path (lots of requests), so fetching the id either by the setting or by the name "Bounce" might slow down (it's already slow)

{
        "id": "8191",
        "option_group_id": "2",
        "label": "Bounce",
        "value": "58",
        "name": "Bounce",
        "filter": "0",
        "is_default": "0",
        "weight": "58",
        "description": "Email was bounced",
        "is_optgroup": "0",
        "is_reserved": "0",
        "is_active": "1"
    }
pbatroff commented 4 years ago

Ok, I understand. How about a configuration file, where the ID is stored and can be read from fast? In that case, patching the source file is not needed.

tttp commented 4 years ago

like adding a constant BOUNCE_TYPE_ACTIVITY in the settings.civicrm.php and do BOUNCE_TYPE_ACTIVITY || 58 ?

pbatroff commented 4 years ago

Yeah, that sounds reasonable for me! If added to documentation then it makes a lot of sense!