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: add verification function to the notification settings for the mailbox #5464

Merged
merged 17 commits into from
Mar 26, 2024

Conversation

LIlGG
Copy link
Member

@LIlGG LIlGG commented Mar 7, 2024

What type of PR is this?

/kind feature
/area ui
/area core
/milestone 2.14.x

What this PR does / why we need it:

为邮件的 通知设置 添加验证的功能。

同时为 formkit 增加了一个新的组件 (verificationForm),用于支持验证,它的定义方式如下:

- $formkit: verificationForm
  action: "http://localhost:8090/verify/user"
  label: 用户验证
  children:
    - $formkit: text
      label: "用户名"
      name: username
      validation: required
    - $formkit: password
      label: "密码"
      name: password
      validation: required

verificationForm 支持 action 属性,当前端数据验证通过时,会将其下所包含的子节点数据发送至 action 所代表的接口上。
按上述示例,则验证数据会提交至 http://localhost:8090/verify/user 进行验证。验证的数据为 {name: xxx, password: xxx}

需要注意的是,verificationForm 只用于包装需要验证的数据,不会破坏原始数据的格式。因此上述数据在提交保存后仍旧为 {name: xxx, password: xxx} 而不会变成 {verificationForm1: {name: xxx, password: xxx}}

How to test it?

  1. 测试邮箱中的 通知设置 新增的验证按钮是否可以正常验证邮箱。
  2. 查看数据是否正常回显

Which issue(s) this PR fixes:

Fixes #4714

Does this PR introduce a user-facing change?

在邮件通知设置中增加了发送测试的功能。

@f2c-ci-robot f2c-ci-robot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Mar 7, 2024
@f2c-ci-robot f2c-ci-robot bot added this to the 2.14.x milestone Mar 7, 2024
@f2c-ci-robot f2c-ci-robot bot added area/ui Issues or PRs related to the Halo UI area/core Issues or PRs related to the Halo Core labels Mar 7, 2024
@f2c-ci-robot f2c-ci-robot bot requested review from Aanko and ruibaby March 7, 2024 07:43
@ruibaby
Copy link
Member

ruibaby commented Mar 7, 2024

hi @longjuan ,有时间可以尝试一下基于此 PR 为 S3 插件的配置表单添加一个验证。

Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 20.48193% with 66 lines in your changes are missing coverage. Please review.

Project coverage is 56.49%. Comparing base (5fdf6c0) to head (f4a0dca).
Report is 81 commits behind head on main.

Files Patch % Lines
...n/halo/app/notification/EmailSenderHelperImpl.java 7.14% 26 Missing ⚠️
...cation/endpoint/EmailConfigValidationEndpoint.java 35.89% 25 Missing ⚠️
...a/run/halo/app/notification/EmailSenderHelper.java 0.00% 9 Missing ⚠️
.../java/run/halo/app/notification/EmailNotifier.java 14.28% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5464      +/-   ##
============================================
- Coverage     56.91%   56.49%   -0.43%     
- Complexity     3319     3320       +1     
============================================
  Files           587      595       +8     
  Lines         18968    19249     +281     
  Branches       1401     1357      -44     
============================================
+ Hits          10795    10874      +79     
- Misses         7594     7813     +219     
+ Partials        579      562      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ruibaby
Copy link
Member

ruibaby commented Mar 7, 2024

hi @LIlGG,在合并此 PR 前,建议同时提交对应文档的修改 PR。

https://docs.halo.run/developer-guide/form-schema

@LIlGG
Copy link
Member Author

LIlGG commented Mar 7, 2024

hi, @guqing ,目前还缺少验证邮箱是否联通的后端接口,有时间的话帮忙补充一下。

@guqing guqing force-pushed the feat/formkit-verify-button branch 2 times, most recently from 22e0c63 to 0556d41 Compare March 7, 2024 09:46
@guqing
Copy link
Member

guqing commented Mar 7, 2024

hi, @guqing ,目前还缺少验证邮箱是否联通的后端接口,有时间的话帮忙补充一下。

已补充

@longjuan
Copy link
Member

longjuan commented Mar 8, 2024

有时间可以尝试一下基于此 PR 为 S3 插件的配置表单添加一个验证。

longjuan/halo-plugin-s3os@e9bba38
https://github.com/longjuan/halo-plugin-s3os/tree/feat/validation-config
发现一个问题,保存后的配置再次编辑时会空白,创建配置时请求data的displayName为空
image
image

@guqing guqing force-pushed the feat/formkit-verify-button branch from 4e8775a to d154ba6 Compare March 11, 2024 07:37
@guqing guqing force-pushed the feat/formkit-verify-button branch from a2949f0 to 0044d34 Compare March 11, 2024 08:00
@LIlGG
Copy link
Member Author

LIlGG commented Mar 11, 2024

发现一个问题,保存后的配置再次编辑时会空白,创建配置时请求data的displayName为空

已进行修复,你可以强制拉取一下最新的代码再进行测试。
另外希望能够测试一下是否能够兼容旧数据,例如先在旧版本上填写配置,然后在适配 verifyForm 之后看看回显是否正常。

@LIlGG LIlGG force-pushed the feat/formkit-verify-button branch from c936f19 to 3426082 Compare March 11, 2024 08:18
@longjuan
Copy link
Member

有个建议:
希望在验证按钮附近提供一个未验证或通过的图标/tag,顶部弹窗有时候注意不到。
当验证后配置又有改变就重新置为未验证。

@longjuan
Copy link
Member

s3 1.8+halo 2.13配置升级到s3 1.8+本pr:无问题
s3 1.8+halo 2.13配置升级到s3测试分支+本pr:无问题

但s3测试分支+本pr 新建策略并编辑会出现名称闪现后消失的问题(旧版本保存的配置没问题)

2024-03-11_20-39-25.mp4

@LIlGG
Copy link
Member Author

LIlGG commented Mar 12, 2024

希望在验证按钮附近提供一个未验证或通过的图标/tag,顶部弹窗有时候注意不到。
当验证后配置又有改变就重新置为未验证。

目前验证按钮是无状态的。另外对于已经保存的内容,再次打开是无法确定是否经过了验证。

不过可以考虑单轮配置阶段增加图标,例如在验证完成之后,加一个验证通过的图标。在修改配置之后,恢复初始状态。

@guqing guqing requested a review from JohnNiang March 18, 2024 04:59
Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

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

IMO,支持验证表单和邮箱验证测试应该是两个功能,如果能够拆分一下提交会不会更好呢?

@LIlGG
Copy link
Member Author

LIlGG commented Mar 20, 2024

IMO,支持验证表单和邮箱验证测试应该是两个功能,如果能够拆分一下提交会不会更好呢?

我认为虽然是可以进行拆分,但此 PR 主要还是用于添加邮箱验证测试这个业务功能,而验证表单是为了满足此业务而产生的组件。例如在写文章列表的同时抽象出一个用于展示列表的公共组件,本质上是同一件事情。

Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/lgtm

@f2c-ci-robot f2c-ci-robot bot added lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 21, 2024
@f2c-ci-robot f2c-ci-robot bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 25, 2024
Copy link

sonarcloud bot commented Mar 25, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.3% Duplication on New Code

See analysis details on SonarCloud

Copy link
Member

@guqing guqing left a comment

Choose a reason for hiding this comment

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

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2024
@LIlGG LIlGG requested a review from ruibaby March 25, 2024 07:39
Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link

f2c-ci-robot bot commented Mar 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ruibaby

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2024
@f2c-ci-robot f2c-ci-robot bot merged commit a281015 into halo-dev:main Mar 26, 2024
8 checks passed
@LIlGG LIlGG deleted the feat/formkit-verify-button branch March 26, 2024 08:10
@ruibaby ruibaby modified the milestones: 2.14.x, 2.14.0 Mar 26, 2024
f2c-ci-robot bot pushed a commit to halo-dev/docs that referenced this pull request Mar 27, 2024
为 FormKit Schema 文档添加 verifyForm 的内容。相关改动查看 halo-dev/halo#5464

/kind documentation

```release-note
None
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/core Issues or PRs related to the Halo Core area/ui Issues or PRs related to the Halo UI kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

建议邮件的“通知设置”添加一个“发送测试”的功能
5 participants