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.

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

don't show USPS warning when USPS lookup disabled #25736

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

MegaphoneJon
Copy link
Contributor

Overview

I spotted this in my last PR's screenshots.

Before

Selection_1798

After

Selection_1800

Technical Details

This happened when removing e-notices from the Smarty. However, this array key will always exist because of CRM_Contact_Import_Form_DataSource::getOptionalQuickFormElements.

@civibot
Copy link

civibot bot commented Mar 6, 2023

(Standard links)

@civibot civibot bot added the master label Mar 6, 2023
@MegaphoneJon
Copy link
Contributor Author

Oh - this also removes these warnings from the page:

    Warning: Trying to access array offset on value of type null in include() (line 117 of /home/jon/local/civicrm-buildkit/app/private/dmaster/default/civicrm/templates_c/en_US/%%88/88C/88CE0B27%%DataSource.tpl.php).
    Warning: Trying to access array offset on value of type null in include() (line 118 of /home/jon/local/civicrm-buildkit/app/private/dmaster/default/civicrm/templates_c/en_US/%%88/88C/88CE0B27%%DataSource.tpl.php).

@eileenmcnaughton
Copy link
Contributor

@MegaphoneJon does this work instead - getOptionalQuickFormElements was an earlier method bug array_value_exists seems cleaner - but both together are bad

--- a/CRM/Contact/Import/Form/DataSource.php
+++ b/CRM/Contact/Import/Form/DataSource.php
@@ -29,19 +29,6 @@ class CRM_Contact_Import_Form_DataSource extends CRM_Import_Form_DataSource {
     return 'contact_import';
   }
 
-  /**
-   * Get any smarty elements that may not be present in the form.
-   *
-   * To make life simpler for smarty we ensure they are set to null
-   * rather than unset. This is done at the last minute when $this
-   * is converted to an array to be assigned to the form.
-   *
-   * @return array
-   */
-  public function getOptionalQuickFormElements(): array {
-    return ['disableUSPS'];
-  }
-

@eileenmcnaughton
Copy link
Contributor

@MegaphoneJon MegaphoneJon force-pushed the remove-usps-warning branch from 1281d35 to be7960a Compare March 7, 2023 19:58
@MegaphoneJon
Copy link
Contributor Author

@eileenmcnaughton That works great. I wasn't sure whether that code was still necessary, I agree this is cleaner.

@eileenmcnaughton
Copy link
Contributor

@MegaphoneJon can you target 5.60? I don't think it needs s stable backport but this is rc-appropriate IMHO as a semi-recent regression

@eileenmcnaughton
Copy link
Contributor

I'm merging this since it's likely no-one else will notice it in the delay between 5.60 & master & getting it merged feels important. We can still choose to backport

@eileenmcnaughton eileenmcnaughton merged commit ccb1916 into civicrm:master Mar 14, 2023
@MegaphoneJon MegaphoneJon deleted the remove-usps-warning branch May 4, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants