This repository has been archived by the owner on May 12, 2023. It is now read-only.
Added more Locales and Currencies. Made som fixes for ProductValueDat… #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…aConverter, when more than one currency and some of them are null
pardahlman
suggested changes
Jul 16, 2017
| Value = date | ||
| }; | ||
| Value = date.ToString("yyyy-MM-dd HH:mm:ss") | ||
| }; |
| Value = date | ||
| }; | ||
| Value = date.ToString("yyyy-MM-dd HH:mm:ss") | ||
| }; |
| Value = date | ||
| }; | ||
| Value = date.ToString("yyyy-MM-dd HH:mm:ss") | ||
| }; |
| @@ -12,7 +12,7 @@ public static UpdatedCriteria Equals(DateTime date) | |||
| return new UpdatedCriteria | |||
| { | |||
| Operator = Operators.Equal, | |||
| Value = date | |||
| Value = date.ToString("yyyy-MM-dd HH:mm:ss") | |||
There was a problem hiding this comment.
I'm a bit hesitant to add a call to ToString here. Is this date format accepted throughout the Akeneo API? If so, we should update the AkeneoSerializerSettings to set DateFormatHandling accordingly.
| return FilterAsync<TModel>(queryString, limit, ct); | ||
| } | ||
|
|
||
| public async Task<PaginationResult<TModel>> FilterAsync<TModel>(string queryString, int limit = 10, CancellationToken ct = default(CancellationToken)) where TModel : ModelBase |
There was a problem hiding this comment.
Just thinking out load here... the signature for FilterAsync has a string argument for queryString, and this overload also accepts a limit that is combined with the queryString... I think this overload is redundant, as the client could create the &limiy= query string herself. Thoughts?
| @@ -62,16 +62,32 @@ public AkeneoClient(Uri apiEndPoint, IAuthenticationClient authClient) : base(ap | |||
| return FilterAsync<TModel>(queryString, ct); | |||
| } | |||
|
|
|||
| public async Task<PaginationResult<TModel>> FilterAsync<TModel>(string queryString, CancellationToken ct = default(CancellationToken)) where TModel : ModelBase | |||
| public Task<PaginationResult<TModel>> SearchAsync<TModel>(IEnumerable<Criteria> criterias, int limit = 10, CancellationToken ct = default(CancellationToken)) where TModel : ModelBase | |||
There was a problem hiding this comment.
I think it makes sense with a limit parameter here. But instead of creating a new method, we should extend the existing one and have a check like
if(limit > 0)
{
// append limit to query string
}
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ProductValueDataConverter is failing when there are more then one currency and one or more of these currencies are null. Sugesting a fix that I have tested and used in my nopCommerce Integration plugin.
Also I have added all Locals and Currencies and made a fix for the dates string used in search. They were failing in my setup.