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 ArticleType's attachmentUrl and CreateOrUpdateArticleReplyFeedback's articleReplyUserId #286

Merged
merged 3 commits into from Jun 30, 2022

Conversation

MrOrz
Copy link
Member

@MrOrz MrOrz commented Jun 25, 2022

Bugfix 1: ArticleType's attachmentUrl field throws error when attachmentHash is empty

The bug was introduced in #284 .

Querying ArticleType's attachmentUrl will trigger Cannot read properties of undefined (reading 'split') error.

圖片

The root cause is that for non-image articles, attachmentHash is null. However, we send null to MediaManager to generate URL for attachmentUrl field.

This PR:

  • Returns null for attachmentUrl if attachmentHash is null
  • Introduces a test case that will fail if this PR is not implemented

Bugfix 2: CreateOrUpdateArticleReplyFeedback records wrong articleReplyUserId on feedback doc

In the image below, the article reply feedback for article 2889dua0k41ro and reply sKR1hn0BnX5-aOa4S29I has its articleReplyUserId being AWA_PbObyCdS-nWhukq-; however, according to the article data, the article reply is actually created by user AVqVwjqQyrDaTqlmmp_a.
圖片

This PR

  • Fixes the typo in the logic that wrongly grabs the first article reply's author as articleReplyUserId
  • Adjust unit test fixture to reproduce the bug and proof that it's fixed

This PR is deployed to staging on 6/25.

@MrOrz MrOrz self-assigned this Jun 25, 2022
@coveralls
Copy link

coveralls commented Jun 25, 2022

Coverage Status

Coverage increased (+0.01%) to 87.544% when pulling b2c3854 on fix-url-resolver into 6540c6c on master.

@MrOrz MrOrz requested a review from nonumpa June 25, 2022 11:15
Also adjust fixture to reproduce the bug
@MrOrz MrOrz changed the title fix ArticleType's attachmentUrl when attachmentHash does not exist Fix ArticleType's attachmentUrl and CreateOrUpdateArticleReplyFeedback's articleReplyUserId Jun 25, 2022
@MrOrz MrOrz merged commit f5e6992 into master Jun 30, 2022
@MrOrz MrOrz deleted the fix-url-resolver branch June 30, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants