Farfetch / blackout

Blackout is the codename for the Farfetch Platform Solutions (FPS) projects. Useful to build e-commerce applications using the FPS APIs and integrating business logic.
MIT License
42 stars 15 forks source link

Rename the analytics API #19

Open nelsonleite opened 2 years ago

nelsonleite commented 2 years ago

Is your proposal related to a problem?

To make the analytics API more consistent and easier to understand.

Describe the solution you'd like

Lifecycle events

The Integration class method setConsent should be renamed to onSetConsent to be consistent with onSetUser method. The loadIntegration event type should be dropped in favour of the onSetUser event that should be called automatically by analytics if the user was set in analytics through the analytics.setUser method.

Analytics

Describe alternatives you've considered

About the loadIntegration event type, there are some integrations that access it to get other data set in analytics. However, none of them is looking at the loadIntegration event type specifically. They are either accessing the context or consent variables, which probably means that the loadData parameter is used not as an event but as a shortcut to get some analytics values, so we could ditch the event property altogether from the loadData parameter as it is not an event per see.

Regarding the changes for analytics.user, analytics.consent and analytics.integration method names, maybe there is no need to have such verbose method names. Since we'll not change what data they return, it will continue to be clear for the developers what these methods are for, and we could simplify the naming a bit by just adding the prefix get{MethodName}, to match the opposite methods we have that have the prefix set{MethodName} for setting those values.

Additional information

Decision log 19 Feb:

ruifcnunes commented 2 years ago

It was decided within the Comms team that this implementation will be postponed to a future version, after the current next is released.