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

feat: 検証可能なシリアル番号を生成 #2

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RabbitProgram
Copy link

作業内容

  • シリアル番号の末尾1桁をチェックディジットとし、リクエスト前に検証を行えるようにした

スクリーンショット

※シリアル番号例:7243-7558-5580

発行 利用
❌️ 間違っている場合
利用
✅️ 正しい場合

補足事項

  • 一般的に使用されている Luhnアルゴリズム を採用しています(luhn-js ライブラリを使用)
  • 早期returnで書いているため、シリアル番号が間違っている場合は /api/coupons APIへのリクエストは行いません

Copy link

vercel bot commented Apr 12, 2024

@RabbitProgram is attempting to deploy a commit to the Yoidea's projects Team on Vercel.

A member of the Team first needs to authorize it.

@RabbitProgram
Copy link
Author

RabbitProgram commented Apr 12, 2024

@yoidea
いつも動画を見させていただいています!
先ほど 肩叩き券の動画 を拝見し、興味深かったのでより安全な形になるようアレンジしてみました。
ご確認のほどよろしくお願いします🙇‍♂️

@rossy0213
Copy link
Member

rossy0213 commented Apr 12, 2024

@RabbitProgram
変更自体は良さそうです。後方互換性はどうしましょうか...すでにランダムのものを発行してるので、そのまま導入すると以前のものが失効になりますね

@RabbitProgram
Copy link
Author

RabbitProgram commented Apr 12, 2024

@rossy0213
コメントありがとうございます!
そうですね...
シリアル番号かパスコードの桁数を増やして、チェックデジット対応後のものかどうかを判別できるようにするのはいかがでしょうか。
(対応後のもののみ検証を行う)

@rossy0213
Copy link
Member

rossy0213 commented Apr 13, 2024

シリアル番号かパスコードの桁数を増やして、チェックデジット対応後のものかどうかを判別できるようにするのはいかがでしょうか。(対応後のもののみ検証を行う)

@RabbitProgram
そうなってくるとメリット薄く感じますね。

@yoidea
実際NotFoundになるケースの割合って出せる? それ次第で重要度が変わってくる気がする。

@yoidea
Copy link
Member

yoidea commented Apr 13, 2024

@rossy0213
すごくざっくり直近1日間のログを見ると

PUT /api/coupons: 3k requests のうち

  • 400: 185 requests
  • 403: 148 requests
  • 404: 187 requests = 6.2%
  • 429: 8 requests

@rossy0213
Copy link
Member

うーん、ちょっと微妙ですね。

@yoidea 13桁にする抵抗ある?
(一応追加するならフォーマットをXXXX-XXXX-XXXX-Xみたいにした方が良さそう)

@yoidea
Copy link
Member

yoidea commented Apr 13, 2024

@yoidea 13桁にする抵抗ある? (一応追加するならフォーマットをXXXX-XXXX-XXXX-Xみたいにした方が良さそう)

人力で入力する以上あまり桁数は増やしたくないところがあるな
#3 がマージされてQRコード等に対応してから再考しても良いかも

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

3 participants