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 #1047 Japanese: Make "events" in the document even clearer #1152

Merged
merged 3 commits into from
Oct 12, 2021

Conversation

wongjas
Copy link
Member

@wongjas wongjas commented Oct 11, 2021

Summary

Applies changes from #1013 and #1046 to the Japanese docs. This pull request fixes #1047. Refer to the issue for details.

Requirements (place an x in each [ ])

@wongjas wongjas requested a review from seratch October 11, 2021 06:41
@CLAassistant
Copy link

CLAassistant commented Oct 11, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #1152 (29156c8) into main (b045c77) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1152   +/-   ##
=======================================
  Coverage   71.71%   71.71%           
=======================================
  Files          15       15           
  Lines        1354     1354           
  Branches      402      402           
=======================================
  Hits          971      971           
  Misses        312      312           
  Partials       71       71           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b045c77...29156c8. Read the comment docs.

Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Thanks for making these updates! Can you check my comments?


1 つだけのワークスペースにインストールされたカスタムアプリであれば `App` 初期化時に単に `token` オプションを使用するだけで OK です。一方で、複数のワークスペースにインストールされる、複数のユーザートークンを使用するといったケースのように、アプリが複数のトークンを処理しなければならない場合があります。このようなケースでは `token` の代わりに `authorize` オプションを使用する必要があります。

`authorize` オプションには、イベントソースを入力値として受け取り、許可された認可されたクレデンシャルを含むオブジェクトを Promise の値として返す関数を指定します。このイベントソースの情報には、 `teamId` (常に存在します)、 `userId`、`conversationId`、`enterpriseId` のような、イベントが誰によって発生させられたか、どこで発生したかに関する情報が含まれます。
`authorize` オプションには、リクエストソースを入力値として受け取り、許可された認可されたクレデンシャルを含むオブジェクトを Promise の値として返す関数を指定します。このリクエストソースの情報には、 `teamId` (常に存在します)、 `userId`、`conversationId`、`enterpriseId` のような、リクエストが誰によって発生させられたか、どこで発生したかに関する情報が含まれます。
Copy link
Member

Choose a reason for hiding this comment

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

@wongjas The term "event source" is not yet changed in the original English version and I think that "event source" is still fine for describing the input of authorize function. https://slack.dev/bolt-js/concepts#authorization Can you keep aligned with the original here for now? The change "リクエストが誰によって発生させられたか" should be kept.

@@ -28,7 +28,7 @@ async function authWithAcme({ payload, client, context, next }) {
// 検索できたらそのユーザ情報でコンテクストを生成
context.user = user;
} catch (error) {
// Acme システム上にユーザが存在しないのでエラーをわたし、イベントプロセスを終了
// Acme システム上にユーザが存在しないのでエラーをわたし、リクエストプロセスを終了
Copy link
Member

Choose a reason for hiding this comment

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

The original one has rooms for improvement. How about improving like this?

Suggested change
// Acme システム上にユーザが存在しないのでエラーをわたし、リクエストプロセスを終了
// Acme システム上にユーザが存在しないパターン。エラーを伝えることとし、リクエストの処理は継続しない

@seratch seratch changed the title Jwong fix 1047 Fix #1047 Japanese: Make "events" in the document even clearer Oct 11, 2021
@seratch
Copy link
Member

seratch commented Oct 11, 2021

@wongjas Can you assign the CLA? Check the above comment by the bot!

@seratch seratch added the docs M-T: Documentation work only label Oct 11, 2021
@seratch seratch added this to the 3.8.0 milestone Oct 11, 2021
@seratch
Copy link
Member

seratch commented Oct 12, 2021

@wongjas Congrats on your first contribution 🎉

@seratch seratch merged commit 7f1ec57 into main Oct 12, 2021
@seratch seratch deleted the jwong_fix_1047 branch October 12, 2021 02:22
srajiang pushed a commit to srajiang/bolt-js that referenced this pull request Oct 14, 2021
slackapi#1152)

* change instances of events to requests in reference to slack requests
* change "request source" back to "event source" and update middleware error as per suggestions
srajiang pushed a commit to srajiang/bolt-js that referenced this pull request Oct 20, 2021
slackapi#1152)

* change instances of events to requests in reference to slack requests
* change "request source" back to "event source" and update middleware error as per suggestions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Japanese: Make "events" in the document even clearer (#1013)
3 participants