dev/core#4213 Make frontend_title, name required for Group entity #26546

eileenmcnaughton · 2023-06-16T07:07:24Z

Overview

dev/core#4213 Make frontend_title, name required for Group entity

This follows up on having done this for contribution_page recently.

It will mean we can consistently refer to frontend_title in tokens and other places where what we really mean is ... frontend_title.

Before

frontend_title, name & title optional at db level, frontend_title optional at form level, title required (name is calculated)

After

Now required at db level, frontend_title required at form level

Technical Details

I'll hit all the same problems as #26259

Comments

civibot · 2023-06-16T07:07:27Z

(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).

eileenmcnaughton · 2023-06-16T20:25:11Z

Fails are because we need to call the parent writeRecord function

This also removes a bunch of tests that are redundant due to the various other levels of api & entity testing & does other minor cleanup like fixing expected & actual variables to be in the right order A cause of test fails for civicrm#26546

colemanw · 2023-07-04T17:50:59Z

@eileenmcnaughton I see this is passing! Is it ready to be merged in your opinion?

eileenmcnaughton · 2023-07-04T22:27:28Z

@colemanw yeah I think so - I'm on the fence about merging it now though because it another day we will cut a new rc - which means on one hand I'd have to re-do the upgrade but on the other it would get more time to find any issues

seamuslee001 · 2023-07-08T23:09:56Z

@eileenmcnaughton are you able to move the upgrade step now that we have cut the new RC?

seamuslee001 · 2023-07-08T23:12:52Z

CRM/Upgrade/Incremental/sql/5.64.alpha1.mysql.tpl

+      UPDATE `civicrm_group`
+      SET `frontend_title_{$locale}` = `title_{$locale}`,
+      `frontend_description_{$locale}` = `description`
+      WHERE `frontend_description_{$locale}` IS NULL OR `frontend_description_{$locale}` = '';


@eileenmcnaughton doesn't this need to be 2 separate queries to handle where frontend_description is NULL or an empty string

@seamuslee001 I don't think so - or rather I'm not following why it would

seamuslee001 · 2023-07-09T23:15:02Z

CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl

+      UPDATE `civicrm_group`
+      SET `frontend_title_{$locale}` = `title_{$locale}`,
+      `frontend_description_{$locale}` = `description`
+      WHERE `frontend_description_{$locale}` IS NULL OR `frontend_description_{$locale}` = '';


@eileenmcnaughton my only thought is that it should be like below where we have 2 separate queries in case frontend_description is NOT NULL i.e. what if for some reason some one had set the frontend_description but not the frontend_title this SQL wouldn't run also why don't you have the same where clause as in L21 ?

ah got it - I pushed the change up

seamuslee001 · 2023-07-09T23:26:48Z

CRM/Upgrade/Incremental/php/FiveSixtyFour.php

@@ -33,9 +33,14 @@ public function upgrade_5_64_alpha1($rev): void {
    $this->addTask('Drop unused civicrm_action_mapping table', 'dropTable', 'civicrm_action_mapping');
    $this->addTask('Update post_URL/cancel_URL in logging tables', 'updateLogging');
    $this->addTask('Add in Everybody ACL Role option value', 'addEveryBodyAclOptionValue');
-    $this->addTask('Fix double json encoding of accepted_credit_cards field in payment processor table', 'fixDoubleEscapingPaymentProcessorCreditCards');


@eileenmcnaughton this should be kept iirc

This follows up on having done this for contribution_page recently. It will mean we can consistently refer to frontend_title in tokens and other places where what we really mean is ... frontend_title.

It now uses tokens not the now-deprecated function

eileenmcnaughton · 2023-07-14T00:05:02Z

@seamuslee001 it passed!!!!!

seamuslee001 · 2023-07-14T00:07:23Z

ok this looks good to me lets merge and we can review

eileenmcnaughton · 2023-07-14T00:12:49Z

phew - got there

demeritcowboy · 2023-07-18T18:43:18Z

CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl

+{/if}
+
+UPDATE civicrm_mailing_component
+SET body_html = REPLACE(body_html, '{welcome.group}', '{group.frontend_title}'),


This doesn't work because it tries to interpret these tokens as smarty.

thanks @demeritcowboy

demeritcowboy · 2023-07-18T20:02:34Z

CRM/Upgrade/Incremental/sql/5.65.alpha1.mysql.tpl

+
+      UPDATE `civicrm_group`
+      SET `frontend_description_{$locale}` = `description`
+      WHERE `frontend_description_{$locale}` IS NULL OR `frontend_description_{$locale}` = '' AND `description` <> '';


My eye just caught this now too. I think it's intended to be (frontend_description_{$locale} IS NULL OR frontend_description_{$locale} = '') AND description <> '' but it gets evaluated as frontend_description_{$locale} IS NULL OR (frontend_description_{$locale} = '' AND description <> ''). In mysql, unlike everything else, AND has a higher precedence than OR: https://dev.mysql.com/doc/refman/8.0/en/operator-precedence.html

Looks like you fixed this too - thanks!

civibot bot added the master label Jun 16, 2023

eileenmcnaughton mentioned this pull request Jun 16, 2023

dev/core#4213 Make frontend_title required #25993

Closed

eileenmcnaughton force-pushed the frontend_group branch from 138e389 to 27fc596 Compare June 16, 2023 08:18

eileenmcnaughton force-pushed the frontend_group branch 3 times, most recently from 75f450a to 03ba6fa Compare June 24, 2023 03:28

eileenmcnaughton mentioned this pull request Jun 24, 2023

Update UFJoin test to not use brittle xml set up #26645

Merged

eileenmcnaughton force-pushed the frontend_group branch 2 times, most recently from 19261bd to 7f3c28b Compare June 24, 2023 21:05

eileenmcnaughton force-pushed the frontend_group branch 2 times, most recently from b302870 to 9aeb31f Compare June 26, 2023 06:17

eileenmcnaughton changed the title ~~[wip] dev/core#4213 Make frontend_title, name required for Group entity~~ dev/core#4213 Make frontend_title, name required for Group entity Jun 30, 2023

eileenmcnaughton force-pushed the frontend_group branch from 9aeb31f to 872b436 Compare June 30, 2023 18:34

eileenmcnaughton mentioned this pull request Jul 3, 2023

Add frontend_title to various data set up places #26721

Merged

eileenmcnaughton force-pushed the frontend_group branch from 872b436 to db449b7 Compare July 3, 2023 21:32

seamuslee001 reviewed Jul 8, 2023

View reviewed changes

eileenmcnaughton force-pushed the frontend_group branch 3 times, most recently from 5a762f2 to f5778c8 Compare July 9, 2023 23:11

seamuslee001 reviewed Jul 9, 2023

View reviewed changes

eileenmcnaughton force-pushed the frontend_group branch from f5778c8 to 0950075 Compare July 9, 2023 23:24

seamuslee001 reviewed Jul 9, 2023

View reviewed changes

eileenmcnaughton force-pushed the frontend_group branch 4 times, most recently from d8478be to ab799d6 Compare July 13, 2023 22:03

eileenmcnaughton added 4 commits July 14, 2023 11:12

Fix PetitionTest on group frontend_title, erm by doing all the stuff

9e5c06a

dev/core#4213 Make frontend_title, name required for Group entity

90a1101

This follows up on having done this for contribution_page recently. It will mean we can consistently refer to frontend_title in tokens and other places where what we really mean is ... frontend_title.

Update the welcome mailing component

8f31a8d

It now uses tokens not the now-deprecated function

mend

e3850c0

eileenmcnaughton force-pushed the frontend_group branch from ab799d6 to e3850c0 Compare July 13, 2023 23:12

seamuslee001 merged commit 798b1c1 into civicrm:master Jul 14, 2023
3 checks passed

seamuslee001 deleted the frontend_group branch July 14, 2023 00:07

demeritcowboy reviewed Jul 18, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev/core#4213 Make frontend_title, name required for Group entity #26546

dev/core#4213 Make frontend_title, name required for Group entity #26546

eileenmcnaughton commented Jun 16, 2023

civibot bot commented Jun 16, 2023

eileenmcnaughton commented Jun 16, 2023

colemanw commented Jul 4, 2023

eileenmcnaughton commented Jul 4, 2023

seamuslee001 commented Jul 8, 2023

seamuslee001 Jul 8, 2023

eileenmcnaughton Jul 9, 2023

seamuslee001 Jul 9, 2023

eileenmcnaughton Jul 9, 2023

seamuslee001 Jul 9, 2023

eileenmcnaughton Jul 9, 2023

eileenmcnaughton commented Jul 14, 2023

seamuslee001 commented Jul 14, 2023

eileenmcnaughton commented Jul 14, 2023

demeritcowboy Jul 18, 2023

demeritcowboy Jul 18, 2023

eileenmcnaughton Jul 21, 2023

demeritcowboy Jul 18, 2023

eileenmcnaughton Jul 21, 2023

dev/core#4213 Make frontend_title, name required for Group entity #26546

dev/core#4213 Make frontend_title, name required for Group entity #26546

Conversation

eileenmcnaughton commented Jun 16, 2023

Overview

Before

After

Technical Details

Comments

civibot bot commented Jun 16, 2023

eileenmcnaughton commented Jun 16, 2023

colemanw commented Jul 4, 2023

eileenmcnaughton commented Jul 4, 2023

seamuslee001 commented Jul 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Jul 14, 2023

seamuslee001 commented Jul 14, 2023

eileenmcnaughton commented Jul 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment