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: Unable to select group viewing permissions #9541

Merged

Conversation

miya
Copy link
Member

@miya miya commented Jan 8, 2025

Task

  • #159935 ページ編集画面で group の閲覧権限を選択できない

@miya miya requested a review from yuki-takei January 8, 2025 03:01
@miya miya self-assigned this Jan 8, 2025
Copy link

changeset-bot bot commented Jan 8, 2025

⚠️ No Changeset found

Latest commit: ac27a76

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@miya miya marked this pull request as ready for review January 8, 2025 03:01
@@ -530,7 +530,7 @@ module.exports = (crowi) => {
} = page;
let isGrantNormalized = false;
try {
const grantedUsersId = grantedUsers.map(ref => getIdForRef(ref));
const grantedUsersId = grantedUsers?.map(ref => getIdForRef(ref));
Copy link
Member Author

Choose a reason for hiding this comment

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

バグの原因は page.grantedUsers が null の時に null 許容の実装になっていないことから 500 エラーが返却されることが原因だった。

@growi/app:dev: 02:28:35.201Z ERROR growi:routes:apiv3:page:
@growi/app:dev:   Error occurred while processing isGrantNormalized. TypeError: Cannot read properties of null (reading 'map')
@growi/app:dev:       at /workspace/growi/apps/app/src/server/routes/apiv3/page/index.ts:533:43
@growi/app:dev:       at processTicksAndRejections (node:internal/process/task_queues:95:5)
@growi/app:dev: GET /_api/v3/page/grant-data?pageId=62a1a77c6efd10033d9504cb 500 7.355 ms - 85

@@ -28,7 +28,7 @@ export type IPage = {
descendantCount: number,
isEmpty: boolean,
grant: PageGrant,
grantedUsers: Ref<IUser>[],
grantedUsers?: Ref<IUser>[],
Copy link
Member Author

Choose a reason for hiding this comment

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

null 許容型に変更

Copy link
Member Author

Choose a reason for hiding this comment

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

type IPage の改修による変更

Copy link
Member Author

Choose a reason for hiding this comment

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

type IPage の改修による変更

Copy link
Member Author

Choose a reason for hiding this comment

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

type IPage の改修による変更

const requests = [
{
updateMany: {
filter: { grantedUsers: null },
Copy link
Member

Choose a reason for hiding this comment

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

filter: { 
  $or: [
    { grantedGroups: null },
    { grantedGroups: { $exists: false } }
  ]
}

にしないと、フィールドが存在しない場合は filter に引っかからないんじゃないかな

Copy link
Member Author

Choose a reason for hiding this comment

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

item: null は null or 存在しないフィールドにマッチするようです。(手元で動作確認もしました)

The { item : null } query matches documents that either contain the item field whose value is null or that do not contain the item field.

https://www.mongodb.com/docs/manual/tutorial/query-for-null-fields/#equality-filter

@miya
Copy link
Member Author

miya commented Jan 9, 2025

@mergify queue

Copy link
Contributor

mergify bot commented Jan 9, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at cc7a2bc

mergify bot added a commit that referenced this pull request Jan 9, 2025
@mergify mergify bot merged commit cc7a2bc into master Jan 9, 2025
21 checks passed
@mergify mergify bot deleted the fix/159936-unable-to-select-group-viewing-permissions branch January 9, 2025 02:31
@github-actions github-actions bot mentioned this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants