Skip to content

Add the support for the query parameter#102

Merged
shalehaha merged 9 commits intodevfrom
offer
Nov 10, 2020
Merged

Add the support for the query parameter#102
shalehaha merged 9 commits intodevfrom
offer

Conversation

@shalehaha
Copy link
Copy Markdown
Contributor

@shalehaha shalehaha commented Nov 6, 2020

Add an optional field query to ExperienceEvent

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 6, 2020

Codecov Report

Merging #102 (b680961) into dev (52982db) will increase coverage by 0.34%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #102      +/-   ##
==========================================
+ Coverage   89.53%   89.87%   +0.34%     
==========================================
  Files          16       16              
  Lines         573      553      -20     
==========================================
- Hits          513      497      -16     
+ Misses         60       56       -4     

@shalehaha shalehaha linked an issue Nov 6, 2020 that may be closed by this pull request
@shalehaha shalehaha requested a review from emdobrin November 6, 2020 20:58
Copy link
Copy Markdown
Contributor

@emdobrin emdobrin left a comment

Choose a reason for hiding this comment

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

Changes look good, left some small comments.

dataDict[Constants.JsonKeys.DATA] = unwrappedData
}

if let query = query {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should also check for empty query map, we should not include it in that case.

XCTAssertEqual("val", eventData["data.testDataDictionary.key"] as? String)
}

func testSendEvent_withXDMDataAndQuery_sendsCorrectRequestEvent() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's move this below testSendEvent_withEmptyXDMSchema_DoesNotSendRequestEvent and include some test cases for nil/empty query data.

XCTAssertNotNil(requestUrl.queryParam("requestId"))
}

func testSendEvent_withXDMDataAndQuery_sendsExEdgeNetworkRequest() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we move this after testSendEvent_withEmptyXDMSchemaAndNilData_doesNotSendExEdgeNetworkRequest

@shalehaha
Copy link
Copy Markdown
Contributor Author

@emdobrin fixed

eventData[Constants.JsonKeys.XDM] = xdm
}

if let query = eventData[Constants.JsonKeys.QUERY] as? [String: Any] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's remove this as it appears to just set the same value back to eventData. I'm seeing the tests pass after removing this statement.

@shalehaha shalehaha merged commit 41ed31a into dev Nov 10, 2020
@kevinlind kevinlind deleted the offer branch October 20, 2022 03:30
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.

add "query" parameter in the event

4 participants