imprv: Can use registration approval request without mail settings #7581
Conversation
…cation' into imprv/119788-120045-can-use-sing-up-registration-setting-without-mail-setting
apps/app/src/components/InAppNotification/InAppNotificationElm.tsx
Outdated
Show resolved
Hide resolved
apps/app/src/components/InAppNotification/InAppNotificationElm.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
このコンポーネント、PageModelNotification と重複が多すぎて冗長。
120632 のスコープに共通化も入れてください。
共通化する際のヒント
- PageModelNotification と UserModelNotification で共通でレンダリングする部分は HoC にすればよさそう
- InAppNotificationElm もあわせてリファクタすべき
- page に閉じるべきものは PageModelNotification に移す
| this.activityEvent.on('updated', async(activity: ActivityDocument, target: IPage, descendantsSubscribedUsers?: Ref<IUser>[]) => { | ||
| // TODO: do not use any type | ||
| // https://redmine.weseek.co.jp/issues/120632 | ||
| this.activityEvent.on('updated', async(activity: ActivityDocument, target: any, users?: Ref<IUser>[]) => { |
There was a problem hiding this comment.
変数名を descendantsSubscribedUsers から users にするメリットある？
There was a problem hiding this comment.
@yuki-takei
sendNotificationToAllAdmins() で adminUsers を渡すようにしたので引数名を汎用的な users にしました。
渡された users (descendantsSubscribedUsers) は createInAppNotification() で操作されます。
もう一度見てみると、createInAppNotificaiton() では const notificationTargetUsers = await activity?.getNotificationTargetUsers(); のようにここで通知する対象のユーザー情報を取得しているコードもあるので、
adminUsers の取得を sendNotificationToAllAdmins() で行うのではなく、createInAppNotification() 内で行えば引数名を変更しなくて済みそうです。
FB修正方針よさそうであればリファクタタスク 120632 のスコープに組み込みます。
| await this.upsertByActivity(users, activity, snapshot); | ||
| await this.emitSocketIo(users); | ||
| return; | ||
| } |
There was a problem hiding this comment.
この場合分け嫌なので 120632 のスコープにリファクタを入れてください
| break; | ||
| default: | ||
| throw new Error(`No serializer found for targetModel: ${doc.targetModel}`); | ||
| } |
There was a problem hiding this comment.
この場合分け嫌なので 120632 のスコープにリファクタを入れてください
…cation' into imprv/119788-120045-can-use-sing-up-registration-setting-without-mail-setting
yuki-takei
merged commit 3284c1f
into
imprv/119788-mail-setting-and-id-password-login-specification
yuki-takei
deleted the
imprv/119788-120045-can-use-sing-up-registration-setting-without-mail-setting
branch
task:https://redmine.weseek.co.jp/issues/120045
Screenshot
01 i18n 修正
Before
After
02 admin ユーザーあてに通知
newUser が Approval pending されている
inappnotifications ドキュメント