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

[BUG] Inconsistency between event-mappings.json and event.schema.json position property #74

Open
russcam opened this issue Dec 13, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@russcam
Copy link

russcam commented Dec 13, 2024

The events-mapping.json defines the "position" property as

"position": {
"properties": {
"ordinal": { "type": "integer" },
"x": { "type": "integer" },
"y": { "type": "integer" },
"page_depth": { "type": "integer" },
"scroll_depth": { "type": "integer" },
"trail": { "type": "text",
"fields": { "keyword": { "type": "keyword", "ignore_above": 256 }
}
}
}

An example "position" conforming to the mapping might look like

{
    ...
    "event_attributes": {
        "position": {
            "ordinal": 1,
            "x": 248,
            "y": 469
        }
    }
}

However, The event JSON schema defines the "position" as

https://github.com/o19s/ubi/blob/412b2fffb9f27e12b73e5f436434c3ebab58241c/schema/1.2.0/event.schema.json#L145-L191

        "position": {
          "description": "Structure that contains information on the location of the event origin, such as screen x,y coordinates, or the nth object out of 10 results.",
          "type": "object",
          "additionalProperties": true,          
          "oneOf": [
                  {
                    "type": "object",
                    "properties": {
                      "ordinal": {
                        "description": "The nth position of the document on the search results page.",
                        "type": "object",
                        "properties": {
                          "index": {
                            "description": "The position of the document.  For grid layout this would be left to right, ignoring wrapping.",
                            "type": "integer",
                            "examples": [1, 3, 24]
                          }
                        },
                        "required": ["index"]
                      }
                    },
                    "required": ["ordinal"]
                  },
                  {
                    "type": "object",
                    "properties": {
                      "xy": {
                        "description": "The x,y coordinates on the screen for triggering an event.",
                        "$comment": "What about bounding boxes?",
                        "type": "object",
                        "properties": {
                          "x": {
                            "description": "The horizontal location on the page or screen of the event.",
                            "type": "number"
                          },
                          "y": {
                            "description": "The vertical location on the page or screen of the event.",
                            "type": "number"
                          }
                        },
                        "required": ["x", "y"]
                      }
                    },
                    "required": ["xy"]
                  }
                ]
        }   

Both ordinal and xy are defined as objects with properties. An example "position" conforming to the schema might look like

{
    ...
    "event_attributes": {
        "position": {
            "ordinal": {
                "index": 1
            },
            "xy": {
                "x": 248,
                "y": 469
            }
        }
    }
}

Which one of the events-mapping.json or event.schema.json should be considered correct?

@russcam russcam added bug Something isn't working untriaged labels Dec 13, 2024
@epugh
Copy link
Collaborator

epugh commented Dec 17, 2024

@russcam good catch! I am looking at this with @wrigleyDan and we are going back and forth on the value of index in the mix.. if you had other attributes you wanted at the ordinal level, then having attributed index is nice. But that is more complexity, an dyou could always stuff your special info around the concept of ordinal up in the event_attributes layer...

@epugh
Copy link
Collaborator

epugh commented Dec 17, 2024

If we want to remove 'index' from the mix:

{
    ...
    "event_attributes": {
        "position": {   
            "ordinal": 1
            }
        }
    }
}

then we should open a ticket in the https://github.com/o19s/ubi repo and make a decision there.. What is your opinion @russcam ?

@russcam
Copy link
Author

russcam commented Dec 20, 2024

Both position and event_attributes allow additional properties, so I think having

  • ordinal be of type integer
  • x be of type integer
  • y be of type integer

as properties on position would be sufficient, as there's enough flexibility at two levels to provide more data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 🆕 New
Development

No branches or pull requests

2 participants