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

Add support for Mongo #3

Merged
merged 1 commit into from
Dec 9, 2019
Merged

Conversation

kambo-1st
Copy link
Collaborator

No description provided.

Copy link
Contributor

@esler esler left a comment

Choose a reason for hiding this comment

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

Hi few suggestions, they aren't critical so we can move it into the next iteration

"_id" : {"$oid": "700000000000000000000002"},
"_created" : {"$date": "2019-03-28T05:30:17.000Z"},
"name" : "Last name",
"type" : "string"
Copy link
Contributor

Choose a reason for hiding this comment

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

*
* @see http://docs.mongodb.org/manual/reference/mongodb-extended-json/
*/
public static function jsonToJsonp(&$json) {
Copy link
Contributor

Choose a reason for hiding this comment

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

}

//transform data into JSONP format which can handle advanced types (eg. MongoId, MongoDate, etc.)
Json::jsonToJsonp($testData);
Copy link
Contributor

Choose a reason for hiding this comment

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

replace with, https://www.php.net/manual/en/function.mongodb.bson-fromjson.php

generally it would be better to use BSON as working format in this Trait

Copy link
Collaborator Author

@kambo-1st kambo-1st Dec 9, 2019

Choose a reason for hiding this comment

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

I created a proof of concept and tested against our JSON fixtures. I quickly found out, that our fixtures are by no means perfect. They are relaying on relaxed treatment of datetime field, which is not possible with function MongoDB\BSON\fromJSON. I propose, at least for now, to still use jsonToJsonp and migrate our fixtures latter.

@kambo-1st
Copy link
Collaborator Author

As our fixtures are not in the perfect shape(viz #3 (comment)), I propose to move this into latter iteration.

@esler esler merged commit af46d2a into peoplepath:master Dec 9, 2019
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.

None yet

2 participants