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: API to save external links as attachments #6364

Merged
merged 9 commits into from
Aug 26, 2024

Conversation

LIlGG
Copy link
Member

@LIlGG LIlGG commented Jul 24, 2024

What type of PR is this?

/kind api-change
/kind feature
/area core

What this PR does / why we need it:

see #2335

增加将第三方资源转存为附件资源的接口。

/apis/api.console.halo.run/v1alpha1/attachments/-/upload-from-url

UC:

/apis/uc.api.content.halo.run/v1alpha1/attachments/-/upload-from-url

其中参数为

{
  "url": "string",
  "filename": "string",
  "groupName": "string",
  "policyName": "string"
}

How to test it?

测试能否将第三方接口的资源保存至附件中。
测试各类附件,例如图片、视频、文本等。

Does this PR introduce a user-facing change?

增加通过链接转存第三方资源至附件库的接口

@f2c-ci-robot f2c-ci-robot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. area/core Issues or PRs related to the Halo Core labels Jul 24, 2024
@f2c-ci-robot f2c-ci-robot bot requested review from ruibaby and wan92hen July 24, 2024 08:07
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 52.10084% with 57 lines in your changes missing coverage. Please review.

Project coverage is 58.26%. Comparing base (6cd8dc8) to head (00bdf66).
Report is 15 commits behind head on main.

Files Patch % Lines
.../endpoint/uc/content/UcPostAttachmentEndpoint.java 28.57% 45 Missing ⚠️
...app/infra/DefaultReactiveUrlDataBufferFetcher.java 0.00% 4 Missing ⚠️
...n/java/run/halo/app/infra/utils/FileNameUtils.java 0.00% 4 Missing ⚠️
...ension/attachment/endpoint/AttachmentEndpoint.java 92.00% 1 Missing and 1 partial ⚠️
...tension/service/impl/DefaultAttachmentService.java 91.30% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6364      +/-   ##
============================================
+ Coverage     58.18%   58.26%   +0.08%     
- Complexity     3774     3786      +12     
============================================
  Files           651      651              
  Lines         22125    22221      +96     
  Branches       1538     1544       +6     
============================================
+ Hits          12873    12947      +74     
- Misses         8641     8663      +22     
  Partials        611      611              

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

@LIlGG LIlGG force-pushed the feat/add-external-transfer branch from 5b4f691 to d2dcd86 Compare July 25, 2024 04:15
@JohnNiang
Copy link
Member

Hi @LIlGG ,我已经创建了新的 PR 专门用来修改 API 文档的版本,这样可以避免当前 PR 产生一些无关的修改。

See #6378 for more.

待该 PR 合并后,你可以通过重新生成 API 文档和 api-client 来解决冲突。

./gradlew generateOpenApiDocs
make -C ui api-client-gen

@LIlGG
Copy link
Member Author

LIlGG commented Jul 25, 2024

Hi @LIlGG ,我已经创建了新的 PR 专门用来修改 API 文档的版本,这样可以避免当前 PR 产生一些无关的修改。

了解了,等此 PR 合并之后我会重新生成一次 OpenAPI。

@LIlGG LIlGG force-pushed the feat/add-external-transfer branch from d2dcd86 to a1eecd0 Compare July 25, 2024 07:11
@LIlGG
Copy link
Member Author

LIlGG commented Jul 25, 2024

/hold 目前接口还需要有很多需要考虑设计的地方,比如

  • 安全性,对于第三方资源的类型进行限制。
  • 文件大小,对于文件的大小进行限制。
  • 需要对个人中心提供单独的接口。

因此暂时先停止合并,等待其配套工作完成之后再做此功能。

@f2c-ci-robot f2c-ci-robot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 25, 2024
@JohnNiang
Copy link
Member

JohnNiang commented Jul 29, 2024

Hi @LIlGG ,请先解决一下代码格式问题。

@LIlGG
Copy link
Member Author

LIlGG commented Aug 19, 2024

/unhold

对于文件安全性和大小的限制,在 PR #6390 中已经实现。

@f2c-ci-robot f2c-ci-robot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 19, 2024
@LIlGG LIlGG force-pushed the feat/add-external-transfer branch from d9886c5 to eddbf10 Compare August 21, 2024 09:20
@LIlGG LIlGG requested a review from guqing August 21, 2024 09:20
@guqing
Copy link
Member

guqing commented Aug 23, 2024

测试失败了需要修复一下

Copy link

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.

/ping @halo-dev/sig-halo
/approve

@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 Aug 26, 2024
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 the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2024
Copy link

f2c-ci-robot bot commented Aug 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: guqing, JohnNiang, 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:
  • OWNERS [JohnNiang,guqing,ruibaby]

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 merged commit e5bbbb3 into halo-dev:main Aug 26, 2024
8 checks passed
@LIlGG LIlGG deleted the feat/add-external-transfer branch August 26, 2024 06:31
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 kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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.

4 participants