Skip to content

Add requestFileInfo param to MethodsClient#filesUploadV2 method #1081

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 8, 2022
Merged

Add requestFileInfo param to MethodsClient#filesUploadV2 method #1081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions slack-api-client/src/main/java/com/slack/api/methods/impl/FilesUploadV2Helper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.slack.api.methods.response.files.FilesGetUploadURLExternalResponse;
import com.slack.api.methods.response.files.FilesInfoResponse;
import com.slack.api.methods.response.files.FilesUploadV2Response;
import com.slack.api.model.File;
import com.slack.api.util.http.SlackHttpClient;
import lombok.Data;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -106,12 +107,17 @@ public FilesUploadV2Response completeUploads(

result.setFiles(new ArrayList<>());
for (FilesCompleteUploadExternalResponse.FileDetails file : response.getFiles()) {
FilesInfoResponse fileInfo = this.client.filesInfo(r -> r.token(v2Request.getToken()).file(file.getId()));
underlyingException.getFileInfoResponses().add(fileInfo);
if (!fileInfo.isOk()) {
throw underlyingException;
if (v2Request.isRequestFileInfo()) {
Copy link
Contributor

@WilliamBergamin WilliamBergamin Nov 8, 2022

Choose a reason for hiding this comment

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

NIT: I would tend to try and place this in its own function named something like fetchFile that returns the File object from fileInfo.getFile() or partialFileObject

Copy link
Contributor Author

@seratch seratch Nov 8, 2022

Choose a reason for hiding this comment

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

Thanks for the suggestion. In this case, the part won't be reused elsewhere plus we have to pass v2Request
and underlyingException to the internal method, which is not so simple. So, let me go with the current code.

FilesInfoResponse fileInfo = this.client.filesInfo(r -> r.token(v2Request.getToken()).file(file.getId()));
underlyingException.getFileInfoResponses().add(fileInfo);
if (!fileInfo.isOk()) {
throw underlyingException;
}
result.getFiles().add(fileInfo.getFile());
} else {
File partialFileObject = File.builder().id(file.getId()).title(file.getTitle()).build();
result.getFiles().add(partialFileObject);
}
result.getFiles().add(fileInfo.getFile());
}

result.setOk(true);
Expand Down
6 changes: 6 additions & 0 deletions slack-api-client/src/main/java/com/slack/api/methods/request/files/FilesUploadV2Request.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ public class FilesUploadV2Request implements SlackApiRequest {
*/
private String snippetType;

/**
* Fetches all the file metadata for better v1 compatibility when this property is true.
*/
@Builder.Default
private boolean requestFileInfo = true;

@Data
@Builder
@NoArgsConstructor
Expand Down
8 changes: 8 additions & 0 deletions slack-api-client/src/test/java/test_locally/api/methods/FilesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,17 @@ public void fileUploadV2_file() throws Exception {
r.file(file).filename("sample.txt").title("sample.txt"));
assertThat(response.isOk(), is(true));

response = slack.methods(ValidToken).filesUploadV2(r ->
r.file(file).filename("sample.txt").title("sample.txt").requestFileInfo(false));
assertThat(response.isOk(), is(true));

response = slack.methodsAsync(ValidToken).filesUploadV2(r ->
r.file(file).filename("sample.txt").title("sample.txt")).get();
assertThat(response.isOk(), is(true));

response = slack.methodsAsync(ValidToken).filesUploadV2(r ->
r.file(file).filename("sample.txt").title("sample.txt").requestFileInfo(false)).get();
assertThat(response.isOk(), is(true));
}

@Test
Expand Down
39 changes: 39 additions & 0 deletions slack-api-client/src/test/java/test_with_remote_apis/methods/files_Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,45 @@ public void createLongTextFile_v2_full_args() throws Exception {
);
assertThat(response.getError(), is(nullValue()));
assertThat(response.isOk(), is(true));
assertThat(response.getFile().getTitle(), is("file title"));
assertThat(response.getFile().getName(), is("sample.txt"));

// I know this is a bit long but this can be necessary for test stability
Thread.sleep(10000L);
fileObj = slack.methods(botToken).filesInfo(r -> r.file(response.getFile().getId())).getFile();

assertThat(fileObj.getTitle(), is("file title"));
assertThat(fileObj.getName(), is("sample.txt"));
}
{
ChatDeleteResponse response = slack.methods(botToken).chatDelete(r -> r
.channel(channelId)
.ts(fileObj
.getShares()
.getPublicChannels().get(channelId).get(0)
.getTs())
);
assertThat(response.getError(), is(nullValue()));
assertThat(response.isOk(), is(true));
}
}

@Test
public void createLongTextFile_v2_no_file_info() throws Exception {
com.slack.api.model.File fileObj;
{
FilesUploadV2Response response = slack.methods(botToken).filesUploadV2(r -> r
.content("test")
.channel(channelId)
.initialComment("initial comment")
.filename("sample.txt")
.title("file title")
.requestFileInfo(false) // Added
);
assertThat(response.getError(), is(nullValue()));
assertThat(response.isOk(), is(true));
assertThat(response.getFile().getTitle(), is("file title"));
assertThat(response.getFile().getName(), is(nullValue())); // this must be absent

// I know this is a bit long but this can be necessary for test stability
Thread.sleep(10000L);
Expand Down