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

Standardize DataSource file for Contribution & Membership #23186

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 13, 2022

Overview

Standardize DataSource file for Contribution & Membership
I've gone through the Datasource.tpl files for Contribution & Membership imports and made them the same - the next step is actually to remove them and use a shared file (& then fix a couple of bugs) but I set out to reconcile them first

Before

image

image

image

image

After

Note - I tested the membership html & contribution html in an online validator

image

image

image

image

Technical Details

Most of the noise is formatting - however notable changes

  1. Class standardisation across fields - e.g
    <div class="crm-block crm-form-block crm-activity-import-uploadfile-form-block">
    becomes

<div class="crm-block crm-form-block crm-import-datasource-form-block">

  1. Re-use of strings - e.g
    {ts 1=$importEntity 2= $importEntities}The %1 Import Wizard allows you to easily upload %2 from other applications into CiviCRM.{/ts}
    {ts}Files to be imported must be in the 'comma-separated-values' format (CSV) and must contain data needed to match an existing contact in your CiviCRM database.{/ts} {help id='upload'}
  1. standisation of html around membership import turned out to fix a shoreditch formatting bug :-)

@colemanw - can you confirm the strings approach is OK - if we want to make any further text of class changes now would be the time - I'm gonna check the other tpls for anything that doesn't fit this pattern & consolidate to 1 file

Comments

@civibot
Copy link

civibot bot commented Apr 13, 2022

(Standard links)

@civibot civibot bot added the master label Apr 13, 2022
* @return string
*/
protected function getTranslatedEntity(): string {
return '';
Copy link
Member

Choose a reason for hiding this comment

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

@eileenmcnaughton your approach to strings looks good, but you could actually simplify it further and not require each child class to override the getTranslatedEntity/Entities functions. This ought to work:

Suggested change
return '';
return Civi\Api4\Utils\CoreUtil::getInfoItem($this::IMPORT_ENTITY, 'title');

And the other one can do the same thing using 'title_plural'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colemanw that works - it does change the capitalisation - but I think that is acceptable

image

@colemanw
Copy link
Member

Did a light r-run and the new strings appear ok.

@colemanw colemanw merged commit 2d8aa03 into civicrm:master Apr 15, 2022
@eileenmcnaughton eileenmcnaughton deleted the import_more branch April 15, 2022 02:48
@eileenmcnaughton
Copy link
Contributor Author

thanks @colemanw

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