-
Notifications
You must be signed in to change notification settings - Fork 43
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 in attachment proccess #321
base: bug/318-file-uploads
Are you sure you want to change the base?
Changes from 10 commits
8304c3b
8849d70
85869d2
6cb873d
de63388
1d96056
7d015a8
677802f
ecb636c
7d674ba
302af22
1bd2719
60490be
91dfa79
24885f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,12 +8,12 @@ namespace Manatee.Trello | |
/// </summary> | ||
public interface IBoardBackgroundCollection : IReadOnlyCollection<IBoardBackground> | ||
{ | ||
/// <summary> | ||
/// Adds a custom board background. | ||
/// </summary> | ||
/// <param name="data">The byte data of the file to attach.</param> | ||
/// <param name="ct">(Optional) A cancellation token for async processing.</param> | ||
/// <returns>The newly created <see cref="IBoardBackground"/>.</returns> | ||
Task<IBoardBackground> Add(byte[] data, CancellationToken ct = default); | ||
/// <summary> | ||
/// Adds a custom board background. | ||
/// </summary> | ||
/// <param name="filePath">The path of the file to attach.</param> | ||
/// <param name="ct">(Optional) A cancellation token for async processing.</param> | ||
/// <returns>The newly created <see cref="IBoardBackground"/>.</returns> | ||
Task<IBoardBackground> Add(string filePath, CancellationToken ct = default); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of changing this, can you just add the new method, please? This will allow us to avoid a breaking change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, its seems to be a great idea |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,13 +8,13 @@ namespace Manatee.Trello | |
/// </summary> | ||
public interface IMemberStickerCollection | ||
{ | ||
/// <summary> | ||
/// Adds a <see cref="ISticker"/> to a <see cref="IMember"/>'s custom sticker set by uploading data. | ||
/// </summary> | ||
/// <param name="data">The byte data of the file to attach.</param> | ||
/// <param name="name">A name for the sticker.</param> | ||
/// <param name="ct">(Optional) A cancellation token for async processing.</param> | ||
/// <returns>The <see cref="ISticker"/> generated by Trello.</returns> | ||
Task<ISticker> Add(byte[] data, string name, CancellationToken ct = default); | ||
/// <summary> | ||
/// Adds a <see cref="ISticker"/> to a <see cref="IMember"/>'s custom sticker set by uploading data. | ||
/// </summary> | ||
/// <param name="filePath">The path of the file to attach.</param> | ||
/// <param name="name">A name for the sticker.</param> | ||
/// <param name="ct">(Optional) A cancellation token for async processing.</param> | ||
/// <returns>The <see cref="ISticker"/> generated by Trello.</returns> | ||
Task<ISticker> Add(string filePath, string name, CancellationToken ct = default); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add an override. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,8 @@ public class RestFile | |
/// </summary> | ||
public string FileName { get; set; } | ||
/// <summary> | ||
/// The file data | ||
/// The file path | ||
/// </summary> | ||
public byte[] ContentBytes { get; set; } | ||
public string FilePath { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here: just add the property. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
using System; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Net; | ||
using System.Net.Http; | ||
|
@@ -176,11 +177,15 @@ private static HttpContent GetContent(WebApiRestRequest request) | |
formData.Add(content, $"\"{parameter.Key}\""); | ||
} | ||
|
||
var byteContent = new ByteArrayContent(request.File); | ||
formData.Add(byteContent, "\"file\"", $"\"{request.FileName}\""); | ||
TrelloConfiguration.Log.Debug($"\tContent: {formData}"); | ||
|
||
return formData; | ||
var fileStream = File.Open(request.File, FileMode.Open); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you will need to check whether raw bytes have been uploaded or just a filename; then act appropriately. |
||
var fileInfo = new FileInfo(request.File); | ||
formData.Add(new StreamContent(fileStream), "\"file\"", string.Format("\"{0}\"", "as" + fileInfo.Extension)); | ||
gregsdennis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
formData.Add(new StringContent("mimeType"), "image/png"); | ||
gregsdennis marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to have some simple functionality that attempts to infer the mime type from the file extension. Not looking for an extensive list; more like a "top 10." |
||
formData.Add(new StringContent("name"), request.FileName); | ||
TrelloConfiguration.Log.Debug($"\tContent: {formData}"); | ||
|
||
return formData; | ||
} | ||
|
||
if (request.Body == null) return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ public IRestRequest Create(string endpoint, IDictionary<string, object> paramete | |
if (parameter.Key == RestFile.ParameterKey) | ||
{ | ||
var rf = (RestFile)parameter.Value; | ||
request.AddFile(parameter.Key, rf.ContentBytes, rf.FileName); | ||
request.AddFile(parameter.Key, rf.FilePath, rf.FileName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to check which attachment method we're using here. I don't want to completely replace the existing bytes method. |
||
} | ||
else | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer not to change the client tests since they're associated with specific issues. Can you create a new test that uses the new code path, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright