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

Scrum 189 fix error of not showing trigger webhook api #49

Merged

Conversation

chungchihhan
Copy link
Collaborator

@chungchihhan chungchihhan commented Dec 16, 2024

Description

因為POST /webhook 這隻 API Request Body有所變更,多了必填的 webhook_type ,因此導致前端無法正常使用,另外加上error的toast來告知 api 出現問題

Type of Change

  • 🐛 Bug Fix(修復 bug)
  • ✨ New Feature(新功能)
  • 💄 UI/UX(介面或使用者體驗優化)
  • 🔨 Refactor(程式碼重構)
  • 📝 Documentation(文件更新)
  • 🔧 Configuration(配置變更)
  • 🚀 Performance(效能優化)
  • ✅ Test(測試相關)
  • 🔒 Security(安全性相關)

Related Issues

SCRUM-189

Testing

  • 可以至 /webhookService 頁面測試

Screenshots/Videos

創建成功:
image
成功複製url :
image
api回傳error或者有必填沒有填寫
(e.g. 如果前端發送的request body缺乏 webhook_type , webhook_type 是必填,因此api response的status會是 error. )
image

Checklist

  • 我已經測試過這些變更
  • 我已經更新相關文件
  • 我的程式碼遵循專案的程式碼規範
  • 我已經處理所有的 console warnings/errors
  • 我已經移除所有的 console.log
  • 我已經新增必要的測試案例
  • 所有現有的測試都能通過

Additional Notes

Copy link

vercel bot commented Dec 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
email-sender ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 17, 2024 7:39am

@poyang1024
Copy link
Collaborator

也是小好奇 failed to create webhook url 的情境有什麼

src/app/ui/create-webhook-form.tsx Show resolved Hide resolved
Comment on lines 96 to 100
if (
response.status === "error" &&
response.errors &&
response.message === "Validation failed"
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

小疑問為什麼有這麼多條件

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @poyang1024 ,這邊的話是為了確保情況是我們要的,但確實有可改進之處, 明天會再修正感謝

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @chungchihhan ,
其實如果合理且是為了完全吻合使用者情境的話是可以這樣寫拉,不過可能就要讓開發者都 sync 一下後端會有多少的 status,errors,message

Comment on lines 110 to 114
// for api error checked
if (
response.status === "error" &&
response.message === "Failed to create webhook. Please try again."
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

小疑問為什麼有這麼多條件(同上)

@chungchihhan
Copy link
Collaborator Author

Hi @poyang1024 , 上述判斷式已經優化修正,感謝

@chungchihhan chungchihhan merged commit 5be0f9c into dev-new Dec 17, 2024
3 checks passed
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.

2 participants