Upload with charset #431
Upload with charset #431
Conversation
|
depends on #348 |
|
@jlevers I think it's OK (got some trouble with markdown auto toc) |
lib/Document.php
Outdated
| public function upload($feedData, ?string $charset = null): void { | ||
| $response = $this->client->put($this->url, [ | ||
| RequestOptions::HEADERS => [ | ||
| "content-type" => $this->contentType, | ||
| "content-type" => self::get_content_type_with_charset($this->contentType, $charset), | ||
| "host" => parse_url($this->url, PHP_URL_HOST), | ||
| ], |
There was a problem hiding this comment.
if we just give the new parameter a default of 'utf-8', we can remove a bunch of code, because then the 'content-type' header can become "$contentType; charset=$charset", and the get_content_type_with_charset method can be removed altogether :)
README.md
Outdated
| ... | ||
| // Create feed document | ||
| $createFeedDocSpec = new Feeds\CreateFeedDocumentSpecification([ | ||
| 'content_type' => SellingPartnerApi\Document::get_content_type_with_charset($feedType['contentType'], $charset)] |
There was a problem hiding this comment.
given my below comment, i think this documentation will need to be changed.
i think that a static method for retrieving a content type with a charset should be put in ContentType.php, something like this:
public static function withCharset(string $contentType, string $charset): string
{
return "{$contentType}; charset={$charset}";
}
|
@jlevers |
|
I'm reverting this until the next major version. It's a breaking change because of the usage differences when creating a new feed document. This has led to a lot of people encountering issues and needing to downgrade (see #486). |
|
oops using a |
|
Would you mind making a new PR with a |
|
OC no pb thx |
Proposition for #336