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

[EN] Change Slingshot to handle multiple updates of person data #3530

Closed
1 task done
JoshuaMaiorino opened this issue Feb 6, 2019 · 3 comments
Closed
1 task done
Labels
Priority: Low Affects a small number of Rock installations and will not be noticed by most users. Type: Enhancement Feature requests.

Comments

@JoshuaMaiorino
Copy link
Contributor

JoshuaMaiorino commented Feb 6, 2019

Prerequisites

Description

There's a bug in the Update portion of slingshot. If after doing an initial import, A person's family is changed in the legacy system, and then an update is attempted, and there's an address within that family it will throw an Insert error when trying to add the new address.

This bug is reported in the slingshot repot here, however the code is in Rock. SparkDevNetwork/Slingshot#3

Steps to Reproduce

  1. Import data from a legacy system
  2. Move an individual from there current family and add them to a new family with an address in the legacy system
  3. Import with Update again

Expected behavior:

The import should add the new family prior to updating the person record.

Actual behavior:

Slingshot attempts to update the person, and add the new address to a family that hasn't been inserted yet, and thus throws an insert error.

Versions

  • Rock Version: [7+]
  • Client Culture Setting: [EN-US]
@JoshuaMaiorino
Copy link
Contributor Author

                    if ( this.ImportUpdateOption == ImportUpdateType.AlwaysUpdate )
                    {
                        Stopwatch stopwatchPersonUpdates = Stopwatch.StartNew();
                        bool wasChanged = UpdatePersonFromPersonImport( person, personImport, attributeValuesLookup, familiesLookup, foreignSystemKey, importDateTime );
                        stopwatchPersonUpdates.Stop();
                        personUpdatesMS += stopwatchPersonUpdates.ElapsedMilliseconds;
                        if ( wasChanged )
                        {
                            personUpdatesCount++;
                        }
                    }

Instead of performing the actual update here, I wonder if it makes sense to store the people to update in a list, and then loop through them after the bulk inserts, which would add the families, prior to trying to add the locations. The family lookup would also have to be updated post inserting the new records. or just how the primary family is gotten would have to be changed.

@nairdo nairdo changed the title Insert Error on second update Insert Error on second Slingshot update May 9, 2019
@nairdo
Copy link
Member

nairdo commented May 9, 2019

I confirmed this today with the team and the original intent of Slingshot is a one time import of data -- it's not intended for continual syncing use. Having said that, it sounds like we may adjust Slingshot in the future to deal with second import attempts (for only a very limited set of data however). Slingshot is not intended to be used as a sync tool.

@nairdo nairdo added Priority: Low Affects a small number of Rock installations and will not be noticed by most users. Type: Enhancement Feature requests. labels May 9, 2019
@nairdo nairdo changed the title Insert Error on second Slingshot update [EN] Change Slingshot to handle multiple updates on person data May 9, 2019
@nairdo nairdo changed the title [EN] Change Slingshot to handle multiple updates on person data [EN] Change Slingshot to handle multiple updates of person data May 9, 2019
@jonedmiston
Copy link
Member

We now have a new Ideas area on our Rock Community website (https://community.rockrms.com/ideas) for posting, discussing and voting on enhancements to Rock. Since this issue is tagged as an enhancement we are closing it and requesting that if you still feel it has merit that you add it as an idea. This allows the full community to speak into it and help us determine it's priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Affects a small number of Rock installations and will not be noticed by most users. Type: Enhancement Feature requests.
Projects
None yet
Development

No branches or pull requests

3 participants