WIP create person with properties early when processing event

[yakkomajuri]

Changes

Playing with the idea of removing all the createPerson calls deeper down in process-event in favor of creating a person with some initial properties early on to make sure it exists.

Putting it out as a WIP PR just to run the entire test stack more easily

Checklist

• [ ] Updated Settings section in README.md, if settings are affected
• [ ] Jest tests

[yakkomajuri]

@macobo what do you think of this?

Currently there are a few places deeper down where we check if a person exists and create it if not.

This PR just makes sure we create a new person right away (with the right properties) if we see an ID we haven't seen before.

It makes sure we only do one query in these cases, so cutting queries when creating a person with properties in half.

The one tradeoff is that if the first event for a person is an $identify event AND the person is being identified to a non-existing person, we'll do a merge which we didn't need to do before.

We can achieve the same effect (person properties set with one query) without this tradeoff, but we'd need to add more special handling. I think things are much cleaner this way, where we always know we're operating on an existing person. Things are quite messy now since we create persons at all sorts of different levels.

Happy to take pushback though. Put this together quickly for feedback.

[macobo]

This is rather complex to understand - I feel like everything is getting tangled more and more and gets harder to understand every time.

Suggestion: Let's use a manager type object instead:

• When processEvent gets called, we do a new PersonManager()
• If createPerson would get called, we instead do personManager.createPerson which will just save that this happened in memory but not actually do it
• When we update distinct ids "merge" or update properties we also just call appropriate personManager methods
• At the end of processEvent, we call personManager.execute(db) which will actually execute the changes - create the user with the right properties and distinct ids etc

We avoid weirdness with different code branches intermingling due to db stuff that's going on. We might also be able to use this pattern for personFound which I think we currently do twice?

[yakkomajuri]

I'm certainly biased, but don't see how this adds complexity. It removes 3 calls to create a person deeper down the call stack and adds the persons properties right away on the first call in processEvent.

Your approach sounds great if all we had to do was create a person. We rely on persons existing though and do other operations to them throughout this code path, so executing this at the end wouldn't work.

Happy to spend some more time thinking about this though. Finishing up the work on set/set_once first

@macobo

[tiina303]

will break into small prs in the merged repo