Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

/**
* Class CRM_Qrcodecheckin_Hook
*
* This class implements hooks for Qrcodecheckin
*/
class CRM_Qrcodecheckin_Hook {

/**
* This hook allows to alter qrcodecheckin tokens.
*
* @param array $values Array of token values for current contactId
* @param int $contactId
* @param bool $handled - Set to TRUE if your hook handled the token values, FALSE to allow default handling
*
* @return mixed
*/
public static function tokenValues(&$values, $contactId, &$handled) {
return CRM_Utils_Hook::singleton()
->invoke(['values', 'contactId', 'handled'], $values, $contactId, $handled, CRM_Utils_Hook::$_nullObject,
CRM_Utils_Hook::$_nullObject, CRM_Utils_Hook::$_nullObject, 'civicrm_qrcodecheckin_tokenValues');
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<?php
use CRM_Qrcodecheckin_ExtensionUtil as E;

class CRM_Qrcodecheckin_Page_QrcodecheckinLanding extends CRM_Core_Page {
var $participant_id = NULL;
var $code = NULL;
Comment on lines +5 to +6

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add proper visibility modifiers and declare missing property.

Class properties are missing visibility modifiers and the $hash property used in the code is not declared.

-  var $participant_id = NULL;
-  var $code = NULL;
+  private $participant_id = NULL;
+  private $code = NULL;
+  private $hash = NULL;
🤖 Prompt for AI Agents
In
wp-content/civi-extensions/net.ourpowerbase.qrcodecheckin-2.1.2/CRM/Qrcodecheckin/Page/QrcodecheckinLanding.php
around lines 5 to 6, add explicit visibility modifiers (such as public,
protected, or private) to the class properties $participant_id and $code. Also,
declare the missing $hash property with an appropriate visibility modifier to
ensure all used properties are properly defined.


public function run() {
// Set the title first.
CRM_Utils_System::setTitle(E::ts('QR Code Check-in page'));

// Now, try to get the participant_id and hash from the URL.
$config = CRM_Core_Config::singleton();
$path = CRM_Utils_Array::value($config->userFrameworkURLVar, $_GET);
// Get everything after /qrcodecheckin/
if (preg_match('#/qrcodecheckin/([0-9]+)/([0-9a-f]+)$#', $path, $matches)) {
$this->participant_id = $matches[1];
$this->hash = $matches[2];
}

// If we don't have both, refuseAccess with message saying URL might be broken.
if (empty($this->participant_id) || empty($this->hash)) {
$this->refuseAccess();
return FALSE;
}

// If we do have them, but they have been altered, send message.
if (!$this->verifyHash()) {
$this->refuseAccess();
return FALSE;
}

// Now we know they check out, let's check permission. If they don't have
// permission to be here, send $pemrission_denied so our template can give
// them a friendly message that doesn't reveal any information.
if (!CRM_Core_Permission::check(QRCODECHECKIN_PERM) && !CRM_Core_Permission::check('edit event participants')) {
$this->assign('has_permission', FALSE);
}
else {
$this->assign('has_permission', TRUE);
CRM_Core_Resources::singleton()->addScriptFile('net.ourpowerbase.qrcodecheckin', 'qrcodecheckin.js');
CRM_Core_Resources::singleton()->addStyleFile('net.ourpowerbase.qrcodecheckin', 'qrcodecheckin.css');
$this->setDetails();
}
parent::run();
}

private function verifyHash() {
$expected_hash = qrcodecheckin_get_code($this->participant_id);
if ($expected_hash != $this->hash) {
CRM_Core_Error::debug_log_message(E::ts("Qrcodecheckin: denied access, hash mis-match for participant id: %1", [ 1 => $this->participant_id]));
return FALSE;
}
return TRUE;
}

private function setDetails() {
$sql = "SELECT title, display_name, st.name as participant_status, fee_level, fee_amount, role_id FROM civicrm_contact c
JOIN civicrm_participant p ON c.id = p.contact_id
JOIN civicrm_event e ON e.id = p.event_id
JOIN civicrm_participant_status_type st ON st.id = p.status_id
WHERE p.id = %0";
$dao = CRM_Core_DAO::executeQuery($sql, array(0 => array($this->participant_id, 'Integer')));
$dao->fetch();
$this->assign('event_title', $dao->title);
$this->assign('display_name', $dao->display_name);
$this->assign('participant_status', $dao->participant_status);
$this->assign('fee_level', $dao->fee_level);
$this->assign('fee_amount', $dao->fee_amount);
$roles = CRM_Core_PseudoConstant::get('CRM_Event_DAO_Participant', 'role_id');
$this->assign('role', $roles[$dao->role_id]);

if ($dao->participant_status == 'Registered') {
$this->assign('update_button', TRUE);
$this->assign('status_class', 'qrcheckin-status-registered');
}
elseif ($dao->participant_status == 'Attended') {
$this->assign('status_class', 'qrcheckin-status-attended');
}
else {
$this->assign('status_class', 'qrcheckin-status-other');
}
}
Comment on lines +58 to +83

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor setDetails() method to follow single responsibility principle.

This method violates the single responsibility principle by handling SQL queries, data assignment, and UI logic. It also has potential SQL injection vulnerabilities despite using parameterized queries correctly.

Consider breaking this into smaller methods:

 private function setDetails() {
+   $participant_data = $this->getParticipantData();
+   $this->assignParticipantDetails($participant_data);
+   $this->setStatusClass($participant_data['participant_status']);
+ }
+
+ private function getParticipantData() {
   $sql = "SELECT title, display_name, st.name as participant_status, fee_level, fee_amount, role_id FROM civicrm_contact c
       JOIN civicrm_participant p ON c.id = p.contact_id
       JOIN civicrm_event e ON e.id = p.event_id
       JOIN civicrm_participant_status_type st ON st.id = p.status_id
       WHERE p.id = %0";
   $dao = CRM_Core_DAO::executeQuery($sql, array(0 => array($this->participant_id, 'Integer')));
   $dao->fetch();
+   return [
+     'title' => $dao->title,
+     'display_name' => $dao->display_name,
+     'participant_status' => $dao->participant_status,
+     'fee_level' => $dao->fee_level,
+     'fee_amount' => $dao->fee_amount,
+     'role_id' => $dao->role_id
+   ];
+ }
+
+ private function assignParticipantDetails($data) {
-   $dao = CRM_Core_DAO::executeQuery($sql, array(0 => array($this->participant_id, 'Integer')));
-   $dao->fetch();
-   $this->assign('event_title', $dao->title);
-   $this->assign('display_name', $dao->display_name);
-   $this->assign('participant_status', $dao->participant_status);
-   $this->assign('fee_level', $dao->fee_level);
-   $this->assign('fee_amount', $dao->fee_amount);
+   $this->assign('event_title', $data['title']);
+   $this->assign('display_name', $data['display_name']);
+   $this->assign('participant_status', $data['participant_status']);
+   $this->assign('fee_level', $data['fee_level']);
+   $this->assign('fee_amount', $data['fee_amount']);
   $roles = CRM_Core_PseudoConstant::get('CRM_Event_DAO_Participant', 'role_id');
-   $this->assign('role', $roles[$dao->role_id]);
+   $this->assign('role', $roles[$data['role_id']]);
+ }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
wp-content/civi-extensions/net.ourpowerbase.qrcodecheckin-2.1.2/CRM/Qrcodecheckin/Page/QrcodecheckinLanding.php
around lines 58 to 83, the setDetails() method mixes SQL querying, data
assignment, and UI logic, violating the single responsibility principle.
Refactor by extracting the SQL query and data fetching into a separate method
that returns participant data, another method to assign data to the template,
and a third method to handle UI logic like setting status classes and update
button visibility. This separation improves maintainability and clarity while
keeping parameterized queries for security.


private function getDisplayName() {
$sql = "SELECT display_name FROM civicrm_contact c JOIN civicrm_participant p ON c.id = p.contact_id
WHERE p.id = %0";
$dao = CRM_Core_DAO::executeQuery($sql, array(0 => array($this->participant_id, 'Integer')));
$dao->fetch();
return $dao->display_name;
}
Comment on lines +85 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unused private method.

Static analysis correctly identifies this method as unused. Dead code should be removed to maintain code quality.

- private function getDisplayName() {
-   $sql = "SELECT display_name FROM civicrm_contact c JOIN civicrm_participant p ON c.id = p.contact_id
-     WHERE p.id = %0";
-   $dao = CRM_Core_DAO::executeQuery($sql, array(0 => array($this->participant_id, 'Integer')));
-   $dao->fetch();
-   return $dao->display_name;
- }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function getDisplayName() {
$sql = "SELECT display_name FROM civicrm_contact c JOIN civicrm_participant p ON c.id = p.contact_id
WHERE p.id = %0";
$dao = CRM_Core_DAO::executeQuery($sql, array(0 => array($this->participant_id, 'Integer')));
$dao->fetch();
return $dao->display_name;
}
🧰 Tools
🪛 PHPMD (2.15.0)

85-91: Avoid unused private methods such as 'getDisplayName'. (Unused Code Rules)

(UnusedPrivateMethod)

🤖 Prompt for AI Agents
In
wp-content/civi-extensions/net.ourpowerbase.qrcodecheckin-2.1.2/CRM/Qrcodecheckin/Page/QrcodecheckinLanding.php
around lines 85 to 91, the private method getDisplayName is not used anywhere in
the codebase. To improve code quality and maintainability, remove this entire
private method including its definition and body.


private function refuseAccess() {
CRM_Core_Error::fatal(E::ts("Woops! The link you clicked on appears to be broken. Please check again and ensure it was not split by a line break.") );
}
Comment on lines +93 to +95

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace fatal error with graceful error handling.

Using CRM_Core_Error::fatal() terminates the entire application. Consider more graceful error handling that allows for better user experience and debugging.

 private function refuseAccess() {
-   CRM_Core_Error::fatal(E::ts("Woops! The link you clicked on appears to be broken. Please check again and ensure it was not split by a line break.") );
+   $this->assign('error_message', E::ts("Woops! The link you clicked on appears to be broken. Please check again and ensure it was not split by a line break."));
+   $this->assign('has_error', TRUE);
+   // Log the error for debugging
+   CRM_Core_Error::debug_log_message(E::ts("QR Code checkin: Access refused for participant_id: %1, hash: %2", [
+     1 => $this->participant_id ?? 'unknown',
+     2 => $this->hash ?? 'unknown'
+   ]));
 }
🤖 Prompt for AI Agents
In
wp-content/civi-extensions/net.ourpowerbase.qrcodecheckin-2.1.2/CRM/Qrcodecheckin/Page/QrcodecheckinLanding.php
around lines 93 to 95, replace the use of CRM_Core_Error::fatal() in the
refuseAccess() method with a more graceful error handling approach. Instead of
terminating the application, implement a user-friendly error message display or
redirect to an error page, allowing the application to continue running and
improving user experience and debugging.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
<?php
use CRM_Qrcodecheckin_ExtensionUtil as E;

/**
* Collection of upgrade steps.
*/
class CRM_Qrcodecheckin_Upgrader extends CRM_Extension_Upgrader_Base {

// By convention, functions that look like "function upgrade_NNNN()" are
// upgrade tasks. They are executed in order (like Drupal's hook_update_N).

/**
* Example: Run an external SQL script when the module is installed.
*
public function install() {
$this->executeSqlFile('sql/myinstall.sql');
}

/**
* Example: Work with entities usually not available during the install step.
*
* This method can be used for any post-install tasks. For example, if a step
* of your installation depends on accessing an entity that is itself
* created during the installation (e.g., a setting or a managed entity), do
* so here to avoid order of operation problems.
*/
// public function postInstall() {
// $customFieldId = civicrm_api3('CustomField', 'getvalue', array(
// 'return' => array("id"),
// 'name' => "customFieldCreatedViaManagedHook",
// ));
// civicrm_api3('Setting', 'create', array(
// 'myWeirdFieldSetting' => array('id' => $customFieldId, 'weirdness' => 1),
// ));
// }

/**
* Example: Run an external SQL script when the module is uninstalled.
*/
// public function uninstall() {
// $this->executeSqlFile('sql/myuninstall.sql');
// }

/**
* Example: Run a simple query when a module is enabled.
*/
// public function enable() {
// CRM_Core_DAO::executeQuery('UPDATE foo SET is_active = 1 WHERE bar = "whiz"');
// }

/**
* Example: Run a simple query when a module is disabled.
*/
// public function disable() {
// CRM_Core_DAO::executeQuery('UPDATE foo SET is_active = 0 WHERE bar = "whiz"');
// }

/**
* Example: Run a couple simple queries.
*
* @return TRUE on success
* @throws Exception
*/
public function upgrade_5200() {
$this->ctx->log->info('Applying update 5200');
$domains = \Civi\Api4\Domain::get(FALSE)
->setLimit(25)
->execute();
foreach ($domains as $domain) {
$qrcode_event = \Civi::settings($domain['id'])->get('default_qrcode_checkin_event');
if ($qrcode_event) {
\Civi::settings($domain['id'])->set('qrcode_events', [$qrcode_event]);
}
}
CRM_Core_DAO::executeQuery('DELETE FROM civicrm_setting WHERE name = "default_qrcode_checkin_event"');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Direct SQL execution violates CiviCRM best practices!

Using raw SQL queries bypasses CiviCRM's API layer and could cause issues with caching or hooks.

Use the Settings API to remove the old setting:

-CRM_Core_DAO::executeQuery('DELETE FROM civicrm_setting WHERE name = "default_qrcode_checkin_event"');
+// Use API to properly remove the setting across all domains
+foreach ($domains as $domain) {
+  \Civi::settings($domain['id'])->revert('default_qrcode_checkin_event');
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
wp-content/civi-extensions/net.ourpowerbase.qrcodecheckin-2.1.2/CRM/Qrcodecheckin/Upgrader.php
at line 75, replace the direct SQL DELETE query with the CiviCRM Settings API
method to remove the "default_qrcode_checkin_event" setting. This ensures proper
cache clearing and hook triggering by using the API instead of raw SQL.

return TRUE;
}
Comment on lines +64 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential data loss in domain migration!

The code limits processing to 25 domains without handling cases where more domains exist. This could result in incomplete migration.

Remove the arbitrary limit or implement proper pagination:

 public function upgrade_5200() {
   $this->ctx->log->info('Applying update 5200');
   $domains = \Civi\Api4\Domain::get(FALSE)
-    ->setLimit(25)
+    ->setLimit(0)  // Process all domains
     ->execute();
🤖 Prompt for AI Agents
In
wp-content/civi-extensions/net.ourpowerbase.qrcodecheckin-2.1.2/CRM/Qrcodecheckin/Upgrader.php
around lines 64 to 77, the upgrade_5200 method limits domain processing to 25,
risking incomplete migration if more domains exist. Remove the setLimit(25) call
to process all domains or implement pagination to handle domains in batches,
ensuring all domains are migrated without data loss.



/**
* Example: Run an external SQL script.
*
* @return TRUE on success
* @throws Exception
*/
// public function upgrade_4201() {
// $this->ctx->log->info('Applying update 4201');
// // this path is relative to the extension base dir
// $this->executeSqlFile('sql/upgrade_4201.sql');
// return TRUE;
// }


/**
* Example: Run a slow upgrade process by breaking it up into smaller chunk.
*
* @return TRUE on success
* @throws Exception
*/
// public function upgrade_4202() {
// $this->ctx->log->info('Planning update 4202'); // PEAR Log interface

// $this->addTask(E::ts('Process first step'), 'processPart1', $arg1, $arg2);
// $this->addTask(E::ts('Process second step'), 'processPart2', $arg3, $arg4);
// $this->addTask(E::ts('Process second step'), 'processPart3', $arg5);
// return TRUE;
// }
// public function processPart1($arg1, $arg2) { sleep(10); return TRUE; }
// public function processPart2($arg3, $arg4) { sleep(10); return TRUE; }
// public function processPart3($arg5) { sleep(10); return TRUE; }

/**
* Example: Run an upgrade with a query that touches many (potentially
* millions) of records by breaking it up into smaller chunks.
*
* @return TRUE on success
* @throws Exception
*/
// public function upgrade_4203() {
// $this->ctx->log->info('Planning update 4203'); // PEAR Log interface

// $minId = CRM_Core_DAO::singleValueQuery('SELECT coalesce(min(id),0) FROM civicrm_contribution');
// $maxId = CRM_Core_DAO::singleValueQuery('SELECT coalesce(max(id),0) FROM civicrm_contribution');
// for ($startId = $minId; $startId <= $maxId; $startId += self::BATCH_SIZE) {
// $endId = $startId + self::BATCH_SIZE - 1;
// $title = E::ts('Upgrade Batch (%1 => %2)', array(
// 1 => $startId,
// 2 => $endId,
// ));
// $sql = '
// UPDATE civicrm_contribution SET foobar = whiz(wonky()+wanker)
// WHERE id BETWEEN %1 and %2
// ';
// $params = array(
// 1 => array($startId, 'Integer'),
// 2 => array($endId, 'Integer'),
// );
// $this->addTask($title, 'executeSql', $sql, $params);
// }
// return TRUE;
// }

}
Loading