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

NEXT-12683 - Add replacing data guide #19

Merged
merged 2 commits into from
Jan 25, 2021

Conversation

PaddyS
Copy link

@PaddyS PaddyS commented Jan 13, 2021

No description provided.

@PaddyS PaddyS requested review from mitelg and pweyck January 13, 2021 13:44
@NiklasLimberg NiklasLimberg added the Core @ct-core label Jan 13, 2021
@PaddyS PaddyS requested a review from shyim January 20, 2021 13:35
@JoshuaBehrens
Copy link
Contributor

I like the comparison to show the pitfall. The scenario up front reads a bit colloquial but not totally made up. I just think it is not nice from the docs to tell the reader they "messed" up.

There is an other pitfall I like to have mentioned in the docs somehow. The product_category.repository can't be used to search as it is a MappingEntityDefinition and it will crash. From my POV I'd say: I can just iterate product_category and look for my entry/search for my entry and just update the category id. I don't want to state that you should say: you can't iterate over mapping entities by design. This can be a bit disappointing (hit me already). But you can mention that you can't update it e.g. by the following reason: When you call update you pass the primary key(s). In that case you have two. You can't update primary keys and thus you can't update the entry but have to delete and create it. Additionally you can suggest to iterate the categories from the product entity so they don't think of my iteration case.

@PaddyS
Copy link
Author

PaddyS commented Jan 22, 2021

@JoshuaBehrens Nice suggestion, but imo this rather fits into the reading data / writing data guides.

Will create an issue for that so we don't forget that one, alright?
#41

@PaddyS
Copy link
Author

PaddyS commented Jan 22, 2021

Nevermind, I'll just fix that immediately with this PR as well.

@PaddyS PaddyS force-pushed the next-12683/add-replacing-data-guide branch from 49ee86a to d00ab7e Compare January 22, 2021 15:16
@PaddyS PaddyS merged commit 62b63e2 into master Jan 25, 2021
@PaddyS PaddyS deleted the next-12683/add-replacing-data-guide branch January 25, 2021 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core @ct-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants