feat: implementions of announcement creation function #8715

WNomunomu · 2024-04-12T08:06:41Z

タスク

https://redmine.weseek.co.jp/issues/134084
https://redmine.weseek.co.jp/issues/134085

概要

announcementを作成、永続化できるようにしました。
announcementからinappnotificationを作成、永続化できるようにしました。

変更点

announcementを作成し、dbに保存するapiの実装
作成したapiをクライアントサイドから叩くための関数の実装

セルフチェック

コンフリクト解消したか
余計なコードは残っていないか
適切にメモ化したか
責務の問題はクリアしているか
CIは通っているか
PRの内容は適切にかけているか

…ons-of-announcement-creation-function

…creation-function

changeset-bot · 2024-05-27T02:13:45Z

⚠️ No Changeset found

Latest commit: 45e69ac

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

reg-suit · 2024-05-27T02:26:30Z

reg-suit detected visual differences.

Check this report, and review them.

⚪
⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫⚫
🔵🔵🔵🔵🔵

What do the circles mean?

The number of circles represent the number of changed images.
🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items

How can I change the check status?

If reviewers approve this PR, the reg context status will be green automatically.

…ons-of-announcement-creation-function

miya · 2024-07-19T08:48:43Z

apps/app/src/server/routes/apiv3/announcement.ts

+
+  const loginRequiredStrictly = require('~/server/middlewares/login-required')(crowi);
+
+  router.post('/', loginRequiredStrictly, async(req: CrowiRequest) => {


pageId は req.params で取れる方が良さそう

express-valudator を使ってバリデーションもする

apps/app/src/server/routes/apiv3/announcement.ts

apps/app/src/client/util/announcement-utils.ts

apps/app/src/server/service/announcement.ts

…creation-function

apps/app/src/server/routes/apiv3/announcement.ts

WNomunomu · 2024-08-06T05:05:20Z

apps/app/src/server/routes/apiv3/announcement.ts

+
+    const params: ParamsForAnnouncement = req.body;
+
+    const page = await Page.findById(params.pageId);


req.paramsではpageIdが正しく取得できなかったため、この形にしています。

クライアントのリクエスト方法がパスパラメーターで取得できるかたちになっていないのでは？
https://github.com/weseek/growi/pull/8715/files#diff-16e65c79c9018f437410fdf1211d572707905ec324e3b3129c795555aa364a5cR13

/:id を用いることで、pageIdをパスパラメータで取得できるようにしました。

…ons-of-announcement-creation-function

miya · 2024-08-26T01:28:30Z

apps/app/src/server/routes/apiv3/announcement.ts

+
+    const params: ParamsForAnnouncement = req.body;
+
+    const page = await Page.findById(params.pageId);


クライアントのリクエスト方法がパスパラメーターで取得できるかたちになっていないのでは？
https://github.com/weseek/growi/pull/8715/files#diff-16e65c79c9018f437410fdf1211d572707905ec324e3b3129c795555aa364a5cR13

apps/app/src/server/routes/apiv3/announcement.ts

apps/app/src/interfaces/announcement.ts

…creation-function

…ons-of-announcement-creation-function

miya · 2024-09-10T07:02:10Z

apps/app/src/features/announcement/routes/apiv3/announcement.ts

+      body('isReadReceiptTrackingEnabled').exists().isBoolean(),
+      body('pageId').exists({ checkFalsy: true }),
+      body('receivers').exists({ checkFalsy: true }).isArray(),
+    ],


sender とか pageId など ObjectId でとるものは isMongoId() をつけよう

バリデーションに失敗した時にクライアントにエラーメッセージを返却するために .withMessage() をつけよう

validatorに設定を追加させていただきました。

miya · 2024-09-10T07:06:14Z

apps/app/src/features/announcement/routes/apiv3/announcement.ts

+
+    const params: ParamsForAnnouncement = req.body;
+
+    const { id: pageId } = req.params;


ごめん、そもそも req.body で pageId をとっているのであればそれを使うでよさそう

req.bodyを採用しました。

yuki-takei · 2024-09-24T11:11:24Z

apps/app/src/features/announcement/client/utils/announcement-utils.ts

+    await apiv3Post('/announcement/doAnnouncement', params);
+  }
+  catch (err) {
+    toastError(err);


toastr の表示は view の責務
このユーティリティは DAO/サービス層に分類されるので不適切

yuki-takei · 2024-09-24T11:13:13Z

apps/app/src/features/announcement/routes/apiv3/announcement.ts

+    ],
+  };
+
+  router.post('/doAnnouncement', loginRequiredStrictly, validators.doAnnouncement, async(req: CrowiRequest) => {


URL は kebab-case で

こちら kebab-case にしました。

yuki-takei · 2024-09-24T11:17:00Z

apps/app/src/features/announcement/server/service/announcement.ts

+
+}
+
+module.exports = AnnouncementService;


ExternalAccountService を参考に、singleton instance をこのモジュールで export するようにしてください

singleton instanceをexportするようにしました。

yuki-takei · 2024-09-24T11:18:04Z

apps/app/src/features/announcement/routes/apiv3/announcement.ts

+
+    const activity = await crowi.activityService.createActivity(parametersForActivity);
+
+    crowi.announcementService.doAnnounce(activity, page, params);


announcementService は crowi 経由で取得するのではなくここで直接動的 import する

try-catch してください

エラーの場合はログ出力して 500 status

yuki-takei · 2024-09-24T11:21:09Z

apps/app/src/features/announcement/routes/apiv3/announcement.ts

+
+    const pageId = params.pageId;
+
+    const page = await Page.findById(pageId);


req.user がアクセスできるページでないといけないので、取得はそれを考慮しているメソッドを使ってください

ページが見つからなかった場合は 4xx 系エラーを返す

yuki-takei · 2024-09-24T11:21:42Z

apps/app/src/features/announcement/server/models/announcement.ts

@@ -58,6 +55,12 @@ const AnnouncementSchema = new Schema<AnnouncementDocument>({
  ],
 }, {});

+AnnouncementSchema.statics.createByParameters = async function(parameters): Promise<IAnnouncement> {
+  const announcement = await this.create(parameters) as IAnnouncement;


as を使わず記述してください

as を消去し、修正しました。

yuki-takei · 2024-09-24T11:22:17Z

apps/app/src/features/announcement/server/service/announcement.ts

+
+  insertAnnouncement = async(
+      params: ParamsForAnnouncement,
+  ): Promise<void> => {


private を付けよう

private method にしました。

yuki-takei · 2024-09-24T11:23:04Z

apps/app/src/features/announcement/server/service/announcement.ts

+      },
+    }];
+
+    await Announcement.bulkWrite(operation);


bulkWrite しないといけない理由ある？

yuki-takei · 2024-09-24T11:24:34Z

apps/app/src/server/routes/login.js

-      props.push(...adminUsers);
+      const { notificationTargetUsers } = props;
+
+      notificationTargetUsers.push(...adminUsers);


これは現存する何かしらのバグの修正？

WNomunomu added 8 commits November 13, 2023 12:08

create AnnouncementService

6451ce7

Merge branch 'feat/announcement-function' into feat/134084-implementi…

b2fee4e

…ons-of-announcement-creation-function

create createByParameters method

a2b50b2

create upsertByActivity method and createAnnouncement method

5b5ddb8

add supported model and action to activity

26b3786

create post api for announcement

b9db316

fix login.js

4b9bcc0

setup AnnouncementService in crowi

7bc44a5

github-actions bot added the type/feature label Apr 12, 2024

WNomunomu changed the base branch from master to feat/announcement-function April 12, 2024 08:09

WNomunomu had a problem deploying to VRT April 12, 2024 08:15 — with GitHub Actions Failure

WNomunomu and others added 6 commits April 15, 2024 08:17

add module export of AnnouncementService

331794e

create button for test

3064298

fix type

924d3e7

create user flow

bbf853a

Merge branch 'master' into feat/134084-implementions-of-announcement-…

a6df789

…creation-function

Merge branch 'master' into feat/134084-implementions-of-announcement-…

9f992b9

…creation-function

WNomunomu changed the base branch from feat/announcement-function to master May 24, 2024 00:08

add router.use to index.js

f1b99be

WNomunomu temporarily deployed to VRT May 27, 2024 02:22 — with GitHub Actions Inactive

Merge branch 'feat/announcement-function' into feat/134084-implementi…

12fdce9

…ons-of-announcement-creation-function

WNomunomu changed the base branch from master to feat/announcement-function June 23, 2024 06:49

WNomunomu had a problem deploying to VRT June 23, 2024 06:57 — with GitHub Actions Failure

WNomunomu added 5 commits June 23, 2024 08:00

trivial fix

ba7fad9

clean code

c15a124

clean code

fc4662d

remove code for test

d9f2013

change to use params instead of announcement

bf6a25d

WNomunomu and others added 5 commits June 23, 2024 14:02

add announcement action to action group

dbe2818

fix parameter and add await prefix

c83a629

change target model

c88e3ef

remove comment

ca53ba1

fix an array type

7a28945

miya requested changes Jul 19, 2024

View reviewed changes

WNomunomu added 2 commits August 5, 2024 09:54

Merge branch 'master' into feat/134084-implementions-of-announcement-…

9cd6f18

…creation-function

fix fb

f6de616

WNomunomu changed the base branch from feat/announcement-function to master August 5, 2024 12:00

WNomunomu changed the base branch from master to feat/announcement-function August 5, 2024 12:01

add validators

a8a2761

WNomunomu commented Aug 6, 2024

View reviewed changes

Merge branch 'feat/announcement-function' into feat/134084-implementi…

950f254

…ons-of-announcement-creation-function

miya requested changes Aug 26, 2024

View reviewed changes

WNomunomu added 6 commits September 9, 2024 05:24

Merge branch 'master' into feat/134084-implementions-of-announcement-…

7eb1c6c

…creation-function

implement to enable to get pageId with path parameters

c60ac35

remove unnecessary parts

d560f70

relocate files related to announcement function

bd33ca5

change relative paths

a87b1ca

Merge branch 'feat/announcement-function' into feat/134084-implementi…

615e657

…ons-of-announcement-creation-function

miya reviewed Sep 10, 2024

View reviewed changes

WNomunomu added 2 commits September 12, 2024 07:57

revert the method for retrieving the pageId

1481713

add validation

6dc961f

miya approved these changes Sep 23, 2024

View reviewed changes

miya requested a review from yuki-takei September 23, 2024 22:57

yuki-takei requested changes Sep 24, 2024

View reviewed changes

WNomunomu added 4 commits September 29, 2024 14:30

make insertAnnouncement method private method

7736dde

change the url to kebab-case

f52a5f5

make AnnouncementService singleton

c6893d4

remove "as" statement

45e69ac

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implementions of announcement creation function #8715

feat: implementions of announcement creation function #8715

WNomunomu commented Apr 12, 2024 •

edited

Loading

changeset-bot bot commented May 27, 2024 •

edited

Loading

reg-suit bot commented May 27, 2024

miya Jul 19, 2024

WNomunomu Aug 6, 2024

miya Aug 26, 2024

WNomunomu Sep 9, 2024

miya Aug 26, 2024

miya Sep 10, 2024

WNomunomu Sep 12, 2024

miya Sep 10, 2024

WNomunomu Sep 12, 2024

yuki-takei Sep 24, 2024

yuki-takei Sep 24, 2024

WNomunomu Sep 29, 2024

yuki-takei Sep 24, 2024

WNomunomu Sep 29, 2024

yuki-takei Sep 24, 2024

yuki-takei Sep 24, 2024

yuki-takei Sep 24, 2024

WNomunomu Sep 29, 2024

yuki-takei Sep 24, 2024

WNomunomu Sep 29, 2024

yuki-takei Sep 24, 2024

yuki-takei Sep 24, 2024


		const loginRequiredStrictly = require('~/server/middlewares/login-required')(crowi);

		router.post('/', loginRequiredStrictly, async(req: CrowiRequest) => {


		const params: ParamsForAnnouncement = req.body;

		const page = await Page.findById(params.pageId);


		const params: ParamsForAnnouncement = req.body;

		const { id: pageId } = req.params;


		const activity = await crowi.activityService.createActivity(parametersForActivity);

		crowi.announcementService.doAnnounce(activity, page, params);


		const pageId = params.pageId;

		const page = await Page.findById(pageId);


		}

		module.exports = AnnouncementService;

feat: implementions of announcement creation function #8715

Are you sure you want to change the base?

feat: implementions of announcement creation function #8715

Conversation

WNomunomu commented Apr 12, 2024 • edited Loading

タスク

概要

変更点

セルフチェック

changeset-bot bot commented May 27, 2024 • edited Loading

⚠️ No Changeset found

reg-suit bot commented May 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WNomunomu commented Apr 12, 2024 •

edited

Loading

changeset-bot bot commented May 27, 2024 •

edited

Loading