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

Update files.upload.v2 internals due to server-side improvements #1668

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

seratch
Copy link
Member

@seratch seratch commented Oct 4, 2023

Summary

This pull request updates the internals of files.upload.v2 method to eliminate files.info API calls, which are no longer necessary because the server-side now returns the same metadata as part of files.completeUploadExternal API responses.

See also:

Requirements (place an x in each [ ])

@seratch seratch added enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api` labels Oct 4, 2023
@seratch seratch added this to the web-api@7.0 milestone Oct 4, 2023
@seratch seratch requested a review from a team October 4, 2023 05:36
@seratch seratch self-assigned this Oct 4, 2023
Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

LGTM, I left one suggestion on either deprecating or removing the v2 upload filesInfo argument.

@@ -1884,7 +1884,7 @@ interface FileUpload {

export interface FilesUploadV2Arguments extends FileUploadV2, WebAPICallOptions, TokenOverridable {
file_uploads?: Omit<FileUploadV2, 'channel_id' | 'channels' | 'initial_comment' | 'thread_ts'>[];
request_file_info?: boolean;
request_file_info?: boolean; // since v7, this flag is no longer used
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add a JSdoc description to this parameter with the @deprecated tag, to make clear not to use this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, since we plan on releasing this in a major version, we can drop this argument altogether.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Removing it may prevent smooth upgrade for many people, so I've added a deprecation comment for it.

@seratch seratch merged commit 1374422 into slackapi:main Oct 4, 2023
14 checks passed
@seratch seratch deleted the files-upload-v2-changes branch October 4, 2023 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants