CRM-20935 Remove related profile links when event deleted #10719
Conversation
JKingsnorth
commented
Jul 21, 2017
•
- CRM-20935: Deleting an event does not delete links to profiles it used
| 1 => array($values['table_name'], 'string'), | ||
| 2 => array($id, 'integer'), | ||
| ); | ||
|
|
There was a problem hiding this comment.
I agree that these lines do nothing - but it seems like the intent was to replace the unescaped variables in the SQL query. I think the better option would be to do so.
There was a problem hiding this comment.
OK I'll take a look at this with a view to fixing escaping the variables in the query.
| 'entity_id' => $id, | ||
| ); | ||
| CRM_Core_BAO_UFJoin::deleteAll($ufJoinParams); | ||
|
|
There was a problem hiding this comment.
I was able to replicate the issue and confirm that this successfully fixes the issue.
| FROM civicrm_uf_join | ||
| LEFT JOIN civicrm_event e on civicrm_uf_join.entity_id = e.id | ||
| WHERE (civicrm_uf_join.module = 'CiviEvent' OR civicrm_uf_join.module = 'CiviEvent_Additional') | ||
| AND e.id IS NULL; |
There was a problem hiding this comment.
I tested this manually, and also thought through this. This SQL is exactly right, though of course it will need to be moved to 4.7.25.mysql.tpl.
There was a problem hiding this comment.
I'll move this up =]
|
rc 4.7.25 is due to be cut in one week. If you are able to address fix the mysql by then we can include it. You get a gold star if you also address Jon's comment |
| FROM civicrm_uf_join | ||
| LEFT JOIN civicrm_event e on civicrm_uf_join.entity_id = e.id | ||
| WHERE (civicrm_uf_join.module = 'CiviEvent' OR civicrm_uf_join.module = 'CiviEvent_Additional') | ||
| AND e.id IS NULL; |
There was a problem hiding this comment.
Just testing this now before pushing up the changes
| @@ -1031,7 +1031,6 @@ public static function &getGroupDetail($groupId = NULL, $searchable = NULL, &$ex | |||
| ); | |||
|
|
|||
| // create select | |||
| $select = "SELECT"; | |||
There was a problem hiding this comment.
This is unused, it is overwritten a few lines further down.
CRM/Event/BAO/Event.php
Outdated
| ); | ||
|
|
||
| CRM_Core_DAO::executeQuery($query); | ||
| $query = "DELETE FROM " . $values['table_name'] . " WHERE entity_id = %1"; |
There was a problem hiding this comment.
This is the pattern used elsewhere. We cannot call the table name a 'string' parameter, because it must not be enclosed in single quotes - otherwise the SQL is invalid. But we are checking the entity ID now.
fb8f485 to
5dd5188
Compare
|
Ignore me - I have corrected it to escape both parameters properly. So this is now good for a quick confirmation and merge. |
|
This seems good to me now - although it could do with being squashed into a single commit. |
|
I'm just doing that now |
5dd5188 to
52b54dd
Compare
|
@eileenmcnaughton All cleaned up, sorry for the noise. |
|
Yup, it's good to merge now. |