fix: Shared page is not displayed when skipping SSR #9089
Conversation
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const relatedPage = await Page.findOne({ _id: getIdForRef(shareLink.relatedPage) }); | ||
| const pagePopulateDataToShowRevision = await relatedPage?.populateDataToShowRevision(); | ||
| return res.apiv3({ page: pagePopulateDataToShowRevision }); | ||
| } |
There was a problem hiding this comment.
この API に shared page を返却する処理が書かれていなかったため getServerSideProps ででやっている相当の処理を追加しました。
参考
growi/apps/app/src/pages/share/[[...path]].page.tsx
Lines 235 to 241 in 8fc33a0
There was a problem hiding this comment.
revision の populate はこの下の try-catch block に任せる
| const { | ||
| pageId, path, findAll, revisionId, | ||
| pageId, path, findAll, revisionId, shareLinkId, | ||
| } = req.query; | ||
|
|
||
| if (pageId == null && path == null) { | ||
| return res.apiv3Err(new ErrorV3('Either parameter of path or pageId is required.', 'invalid-request')); | ||
| } |
There was a problem hiding this comment.
validation が足りない
- (pageId or path) と shareLinkId を同時に指定できてはいけない
- shareLinkId が存在する時は pageId or path は指定されていてはいけない(400 を返す)
- isSharedPage が true なら、shareLinkId は not null でなければならない
- さもなくば
ShareLink.findOne()によって取れてはいけないデータが取れてしまう - middleware で保証されているのであれば、res.apiv3Err を返す必要はなく、単に 500 (throw new Error)でよい
- さもなくば
There was a problem hiding this comment.
(pageId or path) と shareLinkId を同時に指定できてはいけない
certify-shared-page middleware では有効な shareLinkId かどうかを判断するために shareLinkId と pageId を取得する必要があるので validation の条件を以下のように修正しました
const isValid = (shareLinkId != null && pageId != null && path == null) || (shareLinkId == null && (pageId != null || path != null));growi/apps/app/src/server/middlewares/certify-shared-page.js
Lines 9 to 13 in e7a94cb
isSharedPage が true なら、shareLinkId は not null でなければならない
こちらは修正しました。
| const relatedPage = await Page.findOne({ _id: getIdForRef(shareLink.relatedPage) }); | ||
| const pagePopulateDataToShowRevision = await relatedPage?.populateDataToShowRevision(); | ||
| return res.apiv3({ page: pagePopulateDataToShowRevision }); | ||
| } |
There was a problem hiding this comment.
revision の populate はこの下の try-catch block に任せる
miya
changed the title
miya
changed the title
| } | ||
|
|
||
| let page; | ||
| let pages; | ||
| try { | ||
| if (pageId != null) { // prioritized | ||
| if (isSharedPage) { | ||
| const shareLink = await ShareLink.findOne({ _id: shareLinkId }).populate('relatedPage'); |
There was a problem hiding this comment.
.populate('relatedPage') いらないんじゃない？
|
@mergify queue |
🛑 There are no queue conditions matchingThere are queue conditions defined in the configuration, but none matches. The pull request has not been embarked. Details:
|
|
@mergify requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at d44cb18 |
Task