Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1108 [REF] use CRM_Core_DAO::executeQuery instead of ->query() #14760

Merged
merged 1 commit into from
Jul 28, 2019
+5 −5
Merged

dev/core#1108 [REF] use CRM_Core_DAO::executeQuery instead of ->query() #14760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions CRM/Mailing/Event/BAO/Unsubscribe.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,10 @@ public static function &unsub_from_mailing($job_id, $queue_id, $hash, $return =
// list.

while (!empty($mailings)) {
$do->query("
$do = CRM_Core_DAO::executeQuery("
SELECT $mg.entity_table as entity_table,
$mg.entity_id as entity_id
FROM $mg
FROM civicrm_mailing_group $mg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton this looks off to me, why are you specifiying the table name here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 so as I understand it & @mlutfy can jump in if I am wrong we don't need to translate the table name - we just call CRM_Core_DAO::executeQuery and by default that has I10n handling on and it deals with it.

The reason THIS code has had to do all this mucking around is because it didn't want to conform & wanted to show independence by calling a retro function that doesn't handle I10n - but sometimes you just don't get to be an individual

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the other thing is what about when your coming via an extern URL? Like from the {mailing.opt_out_url} token. In those cases have we bootstrapped in all the CRM classes or just done require once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - good thinking - we should test!

WHERE $mg.mailing_id IN (" . implode(', ', $mailings) . ")
AND $mg.group_type = 'Include'");

Expand Down Expand Up @@ -258,12 +258,12 @@ public static function &unsub_from_mailing($job_id, $queue_id, $hash, $return =
if ($groupIds || $baseGroupIds) {
$groupIdClause = "AND $group.id IN (" . implode(', ', array_merge($groupIds, $baseGroupIds)) . ")";
}
$do->query("
$do = CRM_Core_DAO::executeQuery("
SELECT $group.id as group_id,
$group.title as title,
$group.description as description
FROM $group
LEFT JOIN $gc
FROM civicrm_group $group
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again why are we specifiying the table name here rather than in $group?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton can you comment on this please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right so my understanding . is it was because we weren't relying on l10n in the query object - but @seamuslee001 believes it might not be available via extern path

LEFT JOIN civicrm_group_contact $gc
ON $gc.group_id = $group.id
WHERE $group.is_hidden = 0
$groupIdClause
Expand Down