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 upload gift card page and a system info modal #481

Closed
wants to merge 15 commits into from

Conversation

ccjaread
Copy link
Contributor

  1. 针对 兑换码功能使用 #457 尝试新增了一个上传giftcard的页面,方便使用docker的用户导入,但限于post的数据量,需要记录小于2k行,个人觉得一般也够用了,有大规模导入需求的可以用其他专门管理api使用量的app来从源头限制用量或是用mongo工具导入。
    上传文件的样例:
    giftcards-sample.csv
image
  1. 针对 【需求】加个弹窗或通知窗口 #463 搞了个弹窗,本来应该放在网站设置页面的,考虑到部分用户使用的是免登陆模式,目前以折衷的形式在public文件夹新增了notice.json作为弹窗的内容,这样build以后还能从服务器替换的方式更新,同时在pubilc文件夹新增了img文件夹用于放一些图片,效果如下,同时在提示词商店边上加了个按钮可以手动弹出。
image
  1. 修改了一些type-check的报错,这部分请各位大佬看看有没有必要,我是发现不改build进行不下去

新增了聊天数量限制功能
回滚了中英文README,更新了新增的代码注释
新增了聊天数量限制功能,完善了代码注释和新增数据库注释
新增了聊天数量限制功能,完善代码和数据库注释
MongoServerError Cannot apply inc to a value of non-numeric type
新增了轻量的礼品卡上传页面和系统通知弹窗
feat:add giftcard upload page and a system info modal
@BobDu
Copy link
Member

BobDu commented Mar 18, 2024

对于完全无关的功能点。能否拆分独立的分支进行开发并分别进行PR ? 这样混在一起会是review&merge变得比较困难

  1. 上传兑换码功能部分看起来很OK。很乐意在拆分出该部分后尽快merge

  2. 如上所述。弹窗内容是否存在 mongo里面会更好一些? 看起来每次打开页面都会触发一次弹窗? 那是否会有点太频繁了。一般这种都是会本地localStorage里面记录一下? 相同内容的弹窗在用户点击关闭后不再主动弹出? (每次更新弹窗内容都记录不同的弹窗内容id)

  3. 原则上新增类型注解一定是更有利于项目后期维护的。但是同样的。在本次PR中掺杂这些无关功能的新增类型注解变更代码难以merge。所以一般这种 refactor的提交推荐是单独功能分支进行的。不修改代码build具体是什么情况? 理论上不应该是这样子。因为现在的主分支代码同样没有这部分类型注解但是可以build的。你可以本地切换到main分支再试一下能否build? 如果同样不可以那基本能确定是本地的部分环境安装有一些特殊的地方。

@BobDu
Copy link
Member

BobDu commented Mar 18, 2024

@ccjaread 有兴趣一起长期参与维护这个项目吗? 可以给我发email加个微信。

@ccjaread
Copy link
Contributor Author

嗯,这个pr我先关了,后面重新提pr

@ccjaread ccjaread closed this Mar 19, 2024
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