Skip to content

UNOMI-366: Implement increment interest event type & action#201

Merged
sergehuber merged 3 commits intoapache:masterfrom
enonic:issue-366
Nov 19, 2020
Merged

UNOMI-366: Implement increment interest event type & action#201
sergehuber merged 3 commits intoapache:masterfrom
enonic:issue-366

Conversation

@anatol-sialitski
Copy link
Contributor

No description provided.

Copy link
Contributor

@sergehuber sergehuber left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. I am requesting an adjustment to help with compatibility with legacy events.

Copy link
Contributor

Choose a reason for hiding this comment

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

In order to make this compatible with old rules, could we change this so that we use a parameter that says where we are retrieving the interests from ? This was we could say : interests or target.properties.interests like it was in old (legacy) page events. See UNOMi-366 for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated my code. Please have a look

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry I wasn't clear enough. What I meant is that until now when a page view event was sent, it would also contain a list of topics that are metadata on the page and that should be used to increment the interests on the profile. Here's an example of such an event:

unomiTracker.initialize({
​scope: 'myScope',
​url: 'http://unomi:8181', // we use an empty URL to make it relative to this page.
​initialPageProperties: {
​path: path,
​pageInfo: {
​destinationURL: location.href,
​tags: ["tag1", "tag2", "tag3"],
​categories: ["category1", "category2", "category3"]
​},
​interests: {
​"interest1": 1,
​"interest2": 2,
​"interest3": 3
​}
​}
​})

I was hoping we could modify the action to also be able to extract the interests from this type of event, which is why I was suggesting to have an action parameter like "eventInterestProperty" : "target.properties.interests"

Copy link
Contributor

@sergehuber sergehuber left a comment

Choose a reason for hiding this comment

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

Thanks Anatol for changing the PR. It now looks good.

I've approved it and will merge it now.

@sergehuber sergehuber merged commit a535d03 into apache:master Nov 19, 2020
@sergehuber sergehuber deleted the issue-366 branch November 19, 2020 08:17
giladw pushed a commit to YotpoLtd/unomi-releases that referenced this pull request Dec 1, 2020
* master: (26 commits)
  Fix typos &
  Fix Jenkins files
  Add branch information in main README
  UNOMI-400 Update documentation to reflect the latest config changes (apache#221)
  UNOMI-400 Fix class hierarchy lookup for property condition evaluator (apache#220)
  Return error to the client when exception is thrown from Elasticsearch impl (apache#202)
  UNOMI-366: Implement increment interest event type & action (apache#201)
  UNOMI-400 Refactoring of hardcoded property accessors (apache#218)
  UNOMI-401 Fix missing base class in SecureFilteringClassLoader (apache#219)
  UNOMI-399: Provide doc inside the custom.system.properties for scripting allow/forbid mechanism (apache#217)
  UNOMI-400 Fix typo bug in PropertyConditionEvaluator & improve unit test (apache#216)
  UNOMI-400 More complete hardcoded property accessor implementation (apache#214)
  UNOMI-401: correctly set ClassLoader before MVEL script execute (apache#215)
  UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing (apache#213)
  UNOMI-399: fix the loading of MVEL allowed script pattern from config, and return null during script execution if the script is filtered out (apache#211)
  Add documentation to new security configuration parameters.
  Add documentation to new security configuration parameters.
  Remove eslint & dependencies as it is not used.
  UNOMI-379_support_addValues (apache#207)
  feat(geo location condition): add support for getting location path f… (#61) (apache#210)
  ...

# Conflicts:
#	persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java
#	persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
#	persistence-elasticsearch/core/src/main/resources/org.apache.unomi.persistence.elasticsearch.cfg
#	services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java
@felipeelia
Copy link

Hey @sergehuber, is there a plan to merge this in any future release?

@sergehuber
Copy link
Contributor

Hi @felipeelia this is currently merged into the master so it is currently scheduled for Unomi 2. As for backports into 1.x I'm not sure.

Actually I was looking at creating a more generic action that could increment any property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments