Skip to content

Conversation

@innerdvations
Copy link
Contributor

What does it do?

removeRestrictedRelations now uses the value of populateCreatorFields to determine if createdBy and updatedBy should be removed, rather than checking the 'admin::user' permission for those creator fields.

Why is it needed?

Creator fields were available in v3, but in v4 createdByand updatedBy fields are no longer available in the API when populateCreatorFields is true, because they are being removed when the user does not have 'admin::user' permissions (which cannot be granted in the admin)

How to test it?

  • Create a content type
  • create an item of that type
  • set the content type config options.populateCreatorFields to true
  • without this PR, the REST API still does not return the createdBy user
  • with this PR, the REST API will return the createdBy user (same functionality as v3)

Unit tests for this functionality are included

Related issue(s)/PR(s)

#12228

There's a separate issue with the GraphQL schema for creator fields that needs to be fixed before it will be available in GraphQL

@innerdvations innerdvations added issue: bug Issue reporting a bug pr: fix This PR is fixing a bug source: core:utils Source is core/utils or utils packages and removed issue: bug Issue reporting a bug labels Jul 20, 2022
@codecov
Copy link

codecov bot commented Jul 20, 2022

Codecov Report

Merging #13819 (042ca86) into master (80c1da6) will increase coverage by 0.04%.
The diff coverage is 47.22%.

❗ Current head 042ca86 differs from pull request most recent head 2b82678. Consider uploading reports for the commit 2b82678 to get more accurate results

@@            Coverage Diff             @@
##           master   #13819      +/-   ##
==========================================
+ Coverage   54.36%   54.40%   +0.04%     
==========================================
  Files        1190     1190              
  Lines       30184    30199      +15     
  Branches     5476     5471       -5     
==========================================
+ Hits        16409    16430      +21     
+ Misses      11999    11995       -4     
+ Partials     1776     1774       -2     
Flag Coverage Δ
front 56.55% <47.82%> (-0.01%) ⬇️
unit 48.90% <46.15%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../core/admin/admin/src/components/LeftMenu/index.js 32.25% <0.00%> (ø)
...ger/pages/EditSettingsView/components/ModalForm.js 30.35% <0.00%> (ø)
packages/core/admin/admin/src/index.js 0.00% <0.00%> (ø)
...in/admin/src/pages/InstalledPluginsPage/Plugins.js 0.00% <ø> (ø)
.../SettingsPage/pages/Users/EditPage/utils/layout.js 0.00% <ø> (ø)
...gsPage/pages/Users/components/SelectRoles/index.js 51.61% <ø> (ø)
.../content-type-builder/admin/src/pages/App/index.js 0.00% <0.00%> (ø)
...ype-builder/admin/src/pages/RecursivePath/index.js 0.00% <0.00%> (ø)
packages/core/database/lib/query/helpers/where.js 9.89% <0.00%> (-0.34%) ⬇️
...min/src/pages/Settings/components/Configuration.js 85.00% <ø> (ø)
... and 9 more

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 80c1da6...2b82678. Read the comment docs.

Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

Nice work!
Some questions 👀

@innerdvations innerdvations requested a review from Convly July 25, 2022 13:25
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

Just a quick question 👀

@Convly Convly added this to the 4.3.0 milestone Jul 25, 2022
@Convly Convly self-assigned this Jul 25, 2022
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for the tests to pass

@Convly Convly merged commit f688c85 into master Jul 26, 2022
@Convly Convly deleted the fix/missing-creator-fields branch July 26, 2022 08:45
@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/v4-fetch-created-by-field/20238/2

@jaballogian
Copy link

Hello, I tried this on Strapi v4.15.5 but didn't work (I added populateCreatorFields = true but the createdBy and updatedBy fields didn't appear on the API response).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: fix This PR is fixing a bug source: core:utils Source is core/utils or utils packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants