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

fix: normalization error on nested relationship #61

Merged
merged 2 commits into from
Apr 28, 2021

Conversation

kiaking
Copy link
Member

@kiaking kiaking commented Apr 26, 2021

A model with more than 2 related models related to the same model was causing a normalization error. It was due to the Schema class was caching the first created schema with its key.

For example, the first relation creates a schema and say its name is proposal_settings. Now the parent of this model is proposal_templates, because it's defined first in Opportunity model.

Next, if the proposals model also depends on proposal_settings, but because the schema for proposal_settings is already created by proposal_templates, it uses the same proposal_settings schema. Now we have problem because the parent model this time should be proposals but it's using the previous proposal_templates.

We've fixed this by caching the schema with model AND parent entity name. So only when the model and the parent name match, we use the cache.

Type of PR:

  • Bugfix
  • Feature
  • Refactor
  • Code style update
  • Build-related changes
  • Test
  • Documentation
  • Other, please describe:

Breaking changes:

  • No
  • Yes

@kiaking kiaking added the bug Something isn't working label Apr 26, 2021
@kiaking kiaking self-assigned this Apr 26, 2021
@codecov
Copy link

codecov bot commented Apr 26, 2021

Codecov Report

Merging #61 (f0e41a9) into master (c319b3a) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #61   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           36        36           
  Lines          834       834           
  Branches       134       134           
=========================================
  Hits           834       834           
Impacted Files Coverage Δ
src/schema/Schema.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c319b3a...f0e41a9. Read the comment docs.

@kiaking kiaking force-pushed the fix-nested-relationship-normalization-error branch from 74c7493 to e65188f Compare April 28, 2021 03:14
@kiaking kiaking merged commit 239db67 into master Apr 28, 2021
@kiaking kiaking deleted the fix-nested-relationship-normalization-error branch April 28, 2021 03:28
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant