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

dev/core#4130 add template support for imports #25808

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 14, 2023

Overview

dev/core#4130 add template support for imports

This creates UserJob Templates in tandem wtih the use of saved civicrm_mapping records for imports.

This provides a nice-to-have functionality for non-Civi-Import imports - ie the import can store details from the initial DataSource screen - anyone who always has to remember to save dates will appreciate that. In addition for Civi-import imports it can store more nuanced defaults for Civi-Import (which works on the Contribution Import)

The templates are loaded when

  1. the url holds a valid template_id - eg. civicrm/contribute/import?reset=1&template_id=2. A link to that url appears on the Summary screen -
    image

or

  1. a Saved Mapping is selected on the DataSource screen. In this case the defaults for the DataSource screen are not re-loaded but the defaults for MapField is. In practice this is invisible when civi-import is not enabled

E.g - DataSource with contact type default loaded

image

E.g - The sort of field you can set defaults for with Civi-import enabled
image

https://lab.civicrm.org/dev/core/-/issues/4130

Before

MappingFields are too blunt to save the options available in Contribution Import with CiviImport enabled or the general options on the DataSource screen.

The goal is to work towards replacing the old civicrm_mapping - which is also inadequate for some soft credit details - with UserJob template records.

After

Records in civicrm_user_job with is_template = ` used to provide templates for full import configurration.

Both the UserJob Template and the SavedMapping are managed in tandem with a soft link based on name - ie
if civicrm_mapping.name = 'saved_map' then civicrm_user_job.name = 'import_saved_map' for the relevant template.

If the UserJob does not exist it will be created when the DataSource form is submitted with savedMapping not empty or when the MapField form is submitted with saveMapping not empty. Since there are not yet any template UserJobs in the database the reverse will not occur. We could instead create on upgrade. At this stage I decided it would be easier to create as needed in the first iteration.

When the Mapping is updated on the MapField form (either by SaveMapping or UpdateMapping being selected the UserJob template will also be updated.

On the Summary screen a link to use the template is included.

Technical Details

*Flow - do an import, saving a saved mapping. *

  • This will import an associated UserJob Template when the Mapping is saved - ie
    image
  • a link to it will be present on the Summary screen allowing
    Flow 2 Use the template from the url

Add template_id =x to the url & process the import....

Flow 3 Update a template loaded by url

Make a change to an import loaded from the url - note even if the change is on the DataSource page it will only be updated if the Update or Save option is chosen on the MapField page

Flow 3 Update a template loaded by SavedMapping field on the DataSource page

In this case no change will be visible with Civi-Import disabled

Comments

@civibot
Copy link

civibot bot commented Mar 14, 2023

(Standard links)

@civibot civibot bot added the master label Mar 14, 2023
@eileenmcnaughton eileenmcnaughton force-pushed the import_template branch 17 times, most recently from f8b0413 to fcb6168 Compare March 17, 2023 04:00
@eileenmcnaughton eileenmcnaughton changed the title [WIP] dev/core#4130 add template support for imports dev/core#4130 add template support for imports Mar 17, 2023
@eileenmcnaughton eileenmcnaughton force-pushed the import_template branch 8 times, most recently from 4021187 to 175a6b1 Compare March 21, 2023 22:23
@eileenmcnaughton eileenmcnaughton force-pushed the import_template branch 5 times, most recently from 87e49b7 to 47b5900 Compare March 22, 2023 03:40
@eileenmcnaughton
Copy link
Contributor Author

test this please

@aydun
Copy link
Contributor

aydun commented Mar 24, 2023

A few things:

  1. There is something odd with the results table after importing:
    image
    The blue looks like clickable links and clicking on them underlines the whole row. The HTML confirms they are not links.

  2. The contributions seem to import correctly. Hurray!

  3. If I click the link to reuse the template, I get the same options showing up, including the Saved Field Mapping. However, everything is changeable except that Saved Field Mapping:
    image
    For consistency, maybe select the saved field as the default, but still changeable.

@eileenmcnaughton
Copy link
Contributor Author

@aydun thanks for looking - I'm gonna check out the links - but re the mapping - the reason it is locked is the 2 are tied together - ie there is a one-to-one between the import template and the mapping (with a view to maybe one day getting rid of the mapping)

@eileenmcnaughton
Copy link
Contributor Author

@aydun that's odd - I just tried to see the psuedo urls with Shoreditch & greenwich & didn't see them ...

image

image

I did make the numbers into real urls with Civi-Import enabled though - cos it's handy

image

@aydun
Copy link
Contributor

aydun commented Mar 25, 2023

@aydun thanks for looking - I'm gonna check out the links - but re the mapping - the reason it is locked is the 2 are tied together - ie there is a one-to-one between the import template and the mapping (with a view to maybe one day getting rid of the mapping)

OK

@aydun that's odd - I just tried to see the psuedo urls with Shoreditch & greenwich & didn't see them ...

Strange - I'm still seeing them on the test site http://core-25808-1xum.test-3.civicrm.org:8001

@eileenmcnaughton eileenmcnaughton force-pushed the import_template branch 2 times, most recently from 4d91f58 to 790e4be Compare March 25, 2023 20:59
@eileenmcnaughton
Copy link
Contributor Author

@aydun yeah - I could see it on that demo site too - I think I fixed it now

@aydun
Copy link
Contributor

aydun commented Mar 27, 2023

Those sneaky little '/' !! Obviously you meant '< / a>' not '< a >' embedded in another '< a >' - duh

All seems ok without Civi-Import enabled.

With Civi-Import enabled, I get:

Deprecated function: json_decode(): Passing null to parameter #1 ($json) of type string is deprecated in Civi\Api4\Event\Subscriber\ImportSubscriber->getImportTableFromEvent() (line 125 of /home/jenkins/bknix-dfl/build/core-25808-3gzg/web/sites/all/modules/civicrm/ext/civiimport/Civi/Api4/Event/Subscriber/ImportSubscriber.php).

on the queue runner page.

After importing, I clicked the row count to get to the SearchKit page. From there, I wasn't sure what I was supposed to be able to do with it ... but I found the fields were editable so I tried changing the currency from GBP to USD. That didn't affect the imported contributions (makes sense), so I tried importing again and got: An error occurred while attempting to import 1 row/s. but no more info.

The DataSource screen currently declares the type so this makes it available on MapField

Minor tidy up/ standardisation on Contact defaultValues

This also allows the parent to override the values, e.g if loaded from a template

Fix BAO to support Template UserJobs

- do not save non-template fields
- delete templates on Mapping deletes

Add support for UserJob templates at the form level

Fix for strict typing
@eileenmcnaughton
Copy link
Contributor Author

I just pushed up a fix to avoid that notice - it would be php version specific so I hadn't hit it

Regarding the landing page - yeah the idea isn't that you would re-import succeeded rows - it's the same SearchKit as for other errors. @colemanw I do want to check in with you on how to make the actions conditional per row - then we could suppress them is the status is not appropriate. I think that is out of scope though. It would be great if edit-in-place could also be conditional. I think we could merge this based on the review & follow up on that

@aydun the main advantage of being able to view the rows after import is you can see the entity it linked through & click through to it, if you wish

@colemanw colemanw merged commit ed3b445 into civicrm:master Mar 27, 2023
@colemanw colemanw deleted the import_template branch March 27, 2023 23:09
@colemanw
Copy link
Member

Per conversation this is mergeable and can do with a followup.

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.

3 participants