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 find one cant have multiple parameters when there is a primary key #5452

Conversation

petersg83
Copy link
Contributor

@petersg83 petersg83 commented Mar 6, 2020

Fix #4901

Description of what you did:

  • move primary key logic from strapi-connector-* to strapi-database
  • queries done with all params now, not only the primaryKey when there's one
  • destructuring ctx.params where it was needed (to avoid unexpected params from koa-router)

Pierre Noël added 2 commits March 6, 2020 09:08
Signed-off-by: Pierre Noël <pierre.noel@strapi.io>
Signed-off-by: Pierre Noël <pierre.noel@strapi.io>

Signed-off-by: Pierre Noël <pierre.noel@strapi.io>
@derrickmehaffy derrickmehaffy changed the title Fix/#4901/find one cant have multiple parameters when there is apk Fix find one cant have multiple parameters when there is apk Mar 6, 2020
@derrickmehaffy
Copy link
Member

@petersg83 can you check on the failing DCO check (need to sign commits)

@derrickmehaffy derrickmehaffy changed the title Fix find one cant have multiple parameters when there is apk [WIP] Fix find one cant have multiple parameters when there is apk Mar 6, 2020
Signed-off-by: Pierre Noël <pierre.noel@strapi.io>

Signed-off-by: Pierre Noël <pierre.noel@strapi.io>
@petersg83 petersg83 force-pushed the fix/#4901/findOneCantHaveMultipleParametersWhenThereIsAPK branch from b600d01 to b5f9795 Compare March 9, 2020 11:16
@petersg83 petersg83 changed the title [WIP] Fix find one cant have multiple parameters when there is apk Fix find one cant have multiple parameters when there is apk Mar 9, 2020
@codecov
Copy link

codecov bot commented Mar 9, 2020

Codecov Report

Merging #5452 into master will increase coverage by 0.06%.
The diff coverage is 52.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5452      +/-   ##
==========================================
+ Coverage   17.73%   17.80%   +0.06%     
==========================================
  Files         687      688       +1     
  Lines       10151    10162      +11     
  Branches     1643     1646       +3     
==========================================
+ Hits         1800     1809       +9     
- Misses       6961     6962       +1     
- Partials     1390     1391       +1     
Flag Coverage Δ
#front 13.15% <20.00%> (+<0.01%) ⬆️
#unit 37.30% <66.66%> (+0.25%) ⬆️
Impacted Files Coverage Δ
packages/strapi/lib/core-api/controller.js 23.07% <0.00%> (ø)
...s/strapi-admin/admin/src/containers/App/reducer.js 60.00% <20.00%> (-3.64%) ⬇️
packages/strapi-database/lib/utils/primary-key.js 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 0b5ba26...9ab43d2. Read the comment docs.

Signed-off-by: Pierre Noël <pierre.noel@strapi.io>
@alexandrebodin alexandrebodin changed the title Fix find one cant have multiple parameters when there is apk Fix find one cant have multiple parameters when there is a primary key Mar 9, 2020
Pierre Noël and others added 2 commits March 10, 2020 16:44
Signed-off-by: Pierre Noël <pierre.noel@strapi.io>
@alexandrebodin alexandrebodin added source: core:database Source is core/database package issue: bug Issue reporting a bug labels Mar 12, 2020
@alexandrebodin alexandrebodin added this to the 3.0.0-beta.19.4 milestone Mar 12, 2020
@alexandrebodin alexandrebodin merged commit 310fc3d into master Mar 13, 2020
@alexandrebodin alexandrebodin deleted the fix/#4901/findOneCantHaveMultipleParametersWhenThereIsAPK branch March 13, 2020 08:42
@marxvn
Copy link

marxvn commented Apr 3, 2020

Cannot edit content in admin-ui when changing primaryKey

@petersg83
Copy link
Contributor Author

Hi,
Can you please open a new issue for that ?

@marxvn
Copy link

marxvn commented Apr 8, 2020

Hi,
Can you please open a new issue for that ?

yep #5741

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug source: core:database Source is core/database package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FindOne can't have multiple parameters if it contains primary key with mongoose
4 participants