Fix/redirection for new individual signup for pu visit#311
Fix/redirection for new individual signup for pu visit#311nishant22029 merged 14 commits intodevelopfrom
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces significant modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- wp-content/themes/goonj-crm/functions.php (3 hunks)
🔇 Additional comments (1)
wp-content/themes/goonj-crm/functions.php (1)
238-242: Verify the correctness of URL parameters in the redirect.Ensure that the values passed to the
sprintffunction match the expected URL parameters. Specifically, confirm that:
- The
'source'parameter is correctly assigned'pu-visit'.- The
'Individual_fields.Creation_Flow'parameter is assigned'office-visit'.- The
'Individual_fields.Source_Processing_Center'parameter is assigned$target_id.It's important to verify that these values are correctly mapped to prevent any unintended behavior during redirection.
| \Civi::log()->info('check',['cjec']); | ||
| $individual = \Civi\Api4\Contact::get( false ) | ||
| ->addSelect( 'source', 'Individual_fields.Creation_Flow', 'email.email', 'phone.phone' ) | ||
| ->addSelect( 'source', 'Individual_fields.Creation_Flow', 'email.email', 'phone.phone', 'Individual_fields.Source_Processing_Center' ) | ||
| ->addJoin( 'Email AS email', 'LEFT' ) | ||
| ->addJoin( 'Phone AS phone', 'LEFT' ) | ||
| ->addWhere( 'email.is_primary', '=', true ) | ||
| ->addWhere( 'phone.is_primary', '=', true ) | ||
| ->addWhere( 'id', '=', absint( $_GET['individualId'] ) ) | ||
| ->setLimit( 1 ) | ||
| ->execute()->single(); | ||
|
|
||
| \Civi::log()->info('check2',['cjec2'=>$individual]); | ||
| $creationFlow = $individual['Individual_fields.Creation_Flow']; | ||
| $source = $individual['source']; | ||
| $sourceProcessingCenter = $individual['Individual_fields.Source_Processing_Center']; | ||
| // var_dump($creationFlow); | ||
| // var_dump($source); | ||
| // die; |
There was a problem hiding this comment.
Remove debugging statements and commented-out code for clean production code.
The code contains debugging statements and commented-out code that should be removed before deploying to production:
-
Logging statements with non-descriptive messages:
\Civi::log()->info('check',['cjec']); \Civi::log()->info('check2',['cjec2'=>$individual]);
-
Commented-out debug code:
// var_dump($creationFlow); // var_dump($source); // die;
Removing these will clean up the code and prevent unnecessary logging.
Apply this diff to remove the debugging code:
-\Civi::log()->info('check',['cjec']);
$individual = \Civi\Api4\Contact::get( false )
->addSelect( 'source', 'Individual_fields.Creation_Flow', 'email.email', 'phone.phone', 'Individual_fields.Source_Processing_Center' )
->addJoin( 'Email AS email', 'LEFT' )
->addJoin( 'Phone AS phone', 'LEFT' )
->addWhere( 'email.is_primary', '=', true )
->addWhere( 'phone.is_primary', '=', true )
->addWhere( 'id', '=', absint( $_GET['individualId'] ) )
->setLimit( 1 )
->execute()->single();
-\Civi::log()->info('check2',['cjec2'=>$individual]);
$creationFlow = $individual['Individual_fields.Creation_Flow'];
$source = $individual['source'];
$sourceProcessingCenter = $individual['Individual_fields.Source_Processing_Center'];
-// var_dump($creationFlow);
-// var_dump($source);
-// die;| case 'office-visit': | ||
| $redirectPath = sprintf( | ||
| '/processing-center/office-visit/details/#?email=%s&phone=%s&Office_Visit.Goonj_Processing_Center=%s&source_contact_id=%s', | ||
| $email, | ||
| $phone, | ||
| $sourceProcessingCenter, | ||
| $individual['id'] | ||
| ); | ||
| break; | ||
|
|
There was a problem hiding this comment.
Initialize $email and $phone before using them in the redirect.
In the 'office-visit' case, the variables $email and $phone are used in the sprintf function but are not defined in the current scope, which will cause undefined variable errors.
Assign $email and $phone from the $individual array before using them:
$email = $individual['email.email'];
$phone = $individual['phone.phone'];Apply this diff to fix the issue:
+ $email = $individual['email.email'];
+ $phone = $individual['phone.phone'];
case 'office-visit':
$redirectPath = sprintf(
'/processing-center/office-visit/details/#?email=%s&phone=%s&Office_Visit.Goonj_Processing_Center=%s&source_contact_id=%s',
$email,
$phone,
$sourceProcessingCenter,
$individual['id']
);
break;…al-signup-for-pu-visit' into fix/redirection-for-new-individual-signup-for-pu-visit rebase with develop
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
wp-content/themes/goonj-crm/functions.php (3)
226-230: LGTM! Consider using constants for flow values.The changes improve the tracking of user registration sources by adding 'source' and 'Individual_fields.Creation_Flow' parameters. This is a good enhancement for better data collection and user flow tracking.
Consider defining constants for the 'Individual_fields.Creation_Flow' values ('pu-visit-contribution' and 'office-visit-contribution') to improve code maintainability and reduce the risk of typos. For example:
define('CREATION_FLOW_PU_VISIT_CONTRIBUTION', 'pu-visit-contribution'); define('CREATION_FLOW_OFFICE_VISIT_CONTRIBUTION', 'office-visit-contribution'); // Then use them in the code: 'Individual_fields.Creation_Flow=%s', CREATION_FLOW_PU_VISIT_CONTRIBUTION,
240-244: LGTM! Consider using constants for flow values (continued).The changes in this segment are consistent with the previous improvements, adding 'source' and 'Individual_fields.Creation_Flow' parameters for better tracking of office visit registrations.
As suggested earlier, consider defining constants for the 'Individual_fields.Creation_Flow' values ('pu-visit' and 'office-visit') to improve code maintainability. For example:
define('CREATION_FLOW_PU_VISIT', 'pu-visit'); define('CREATION_FLOW_OFFICE_VISIT', 'office-visit'); // Then use them in the code: 'Individual_fields.Creation_Flow=%s', CREATION_FLOW_PU_VISIT,
Line range hint
447-504: LGTM with a minor issue. Initialize $email and $phone variables.The changes improve the flexibility of the redirection logic by adding new cases for 'office-visit' and 'office-visit-contribution'. The addition of 'Individual_fields.Source_Processing_Center' to the API query allows for more detailed tracking of individual registrations.
However, there's a potential issue in the new cases. The variables $email and $phone are used but not defined. To fix this, initialize these variables before using them:
$creationFlow = $individual['Individual_fields.Creation_Flow']; $source = $individual['source']; $sourceProcessingCenter = $individual['Individual_fields.Source_Processing_Center']; +$email = $individual['email.email']; +$phone = $individual['phone.phone']; if ( ! $source ) { return; }This will ensure that $email and $phone are properly defined before being used in the sprintf calls.
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (3)
Line range hint
486-489: Check for null before accessing$stateField['id']to prevent potential errorsIn
setIndianStateOptions, if$intentStateFields->first()returnsNULL, accessing$stateField['id']will result in a fatal error. To prevent this, add a null check for$stateFieldbefore accessing its properties.Apply this diff to add the null check:
$stateField = $intentStateFields->first(); + if (!$stateField) { + return; + } $statefieldId = $stateField['id'];
92-93: Ensure consistent naming conventions for directive namesIn the 'campOutcome' tab configuration, the directive name
'afform-admin-Camp-outcome-form'uses an uppercase 'C' in 'Camp'. For consistency with other directives and to prevent potential issues, consider changing it to use lowercase:Apply this diff to update the directive name:
- 'directive' => 'afform-admin-Camp-outcome-form', + 'directive' => 'afform-admin-camp-outcome-form',
Line range hint
486-508: Handle potential empty results when fetching Indian statesIn
setIndianStateOptions, after fetching Indian states, it's possible that$indianStatescould be empty. Consider adding a check to handle this scenario to prevent unexpected behavior.Apply this diff to handle empty state results:
$indianStates = StateProvince::get(FALSE) ->addWhere('country_id.iso_code', '=', 'IN') ->addOrderBy('name', 'ASC') ->execute(); + if ($indianStates->count() === 0) { + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1 hunks)
- wp-content/themes/goonj-crm/functions.php (5 hunks)
🔇 Additional comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
110-110: Ensure modules are correctly loaded in AngularJSWhen adding modules using
addModules($config['module']), ensure that the module names are accurately specified and that the corresponding AngularJS modules exist. This prevents potential errors during runtime due to missing or incorrectly named modules.
91-91: Verify that the module name is correctly registeredEnsure that the module
'afformAdminCampOutcomeForm'is correctly registered and loaded in the AngularJS application. Inconsistent naming or incorrect module registration can lead to runtime errors.Run the following script to search for the module definition:
|
|
||
| $creationFlow = $individual['Individual_fields.Creation_Flow']; | ||
| $source = $individual['source']; | ||
| $sourceProcessingCenter = $individual['Individual_fields.Source_Processing_Center']; |
There was a problem hiding this comment.
let's move this variable inside the cases where we needed it (case 'office-visit' and case 'office-visit-contribution')
nishant22029
force-pushed
the
fix/redirection-for-new-individual-signup-for-pu-visit
branch
from
ce7a827 to
e342662
Compare
nishant22029
deleted the
fix/redirection-for-new-individual-signup-for-pu-visit
branch
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores