Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
DOCSP-39180: Symfony Quick Start #971
DOCSP-39180: Symfony Quick Start #971
Changes from 20 commits
c327520
82ea9e8
157f580
ac04d15
bddcbaa
5465f48
8a5a5be
24ef1ba
3d91667
02e77e6
a490e74
a05e2b7
1cc4dcd
0178ab7
65f7e10
4ce9441
29e3019
b30e78f
1144580
7edd4ee
f55ff27
6756e75
1f5d6fd
c052d89
24a00a9
b536c70
56122ae
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alcaeus: Should an ObjectId type hint be used here? That would warrant importing
MongoDB\BSON\ObjectId
.Even if no typing change is made, there's an extra space on this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be a
string
, as ODM'sId
type converts ObjectIds to strings for better handling in web requests. Off the top of my head I don't know how well Doctrine handles uninitialised properties, so if we want to be cautious it should be a nullable string with default value:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, of course. I want to come out on record here and say that it's been that long since I worked on an app with ODM that I legitimately forgot about that for a hot minute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the field definition was fine, but changing the annotation to
#[ODM\Field]
for theid
field resulted in this error:No identifier/primary key specified for Entity...
Is this expected? Fixed by changing the annotation back to
#[ODM\Id]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[ODM\Id]
is correct. Even though ODM converts ObjectIds to string in the model class, it should be tracked as an identifier.