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

Fixes and changes to JsonModelBinder, OrdersController, Images and Categories. #184

Open
wants to merge 15 commits into
base: nopCommerce_4.10
Choose a base branch
from

Conversation

MaximG1234
Copy link
Contributor

@MaximG1234 MaximG1234 commented Jan 24, 2019

List of changes in this checkin:

  1. Modified JsonModelBinder to support the invocation of attributes which are defined at the class level.
    a. Previously if you added an attribute such as ImageValidation to a DTO object at the class level, validation would never be invoked.
    b. This can be replicated by adding an attribute such as ImageValidation and noting that the Validate() method is never called.
    c. I have resolved this issue by modifying the JsonModelBinder and adding a check for class level attributes.

  2. Modified the OrdersController class to return Shipments for a given order.
    a. Previously if you returned an order by invoking the OrdersController class the Order DTO returned did not contain a way to access Shipments for a given order.
    b. I have resolved this issue by modifing the OrderDto and adding the property Shipments which is a ICollection of ShipmentDto's.

  3. Modified the ImageDto class to inherit from BaseDto.
    a. Previously the ImageDto class did not inherit from the BaseDto class which prevented you obtaining the Id for a given image.

  4. Modified DeleteProduct method to correctly log Activity including stock name.
    a. Previously when invoking the Delete method in ProductsController the activity logged did not correctly provide the stock affected name.

  5. Created new controller ProductPicturesController.
    a. Previously it was impossible to independently edit Product images seperate from the ProductsController.
    b. There was also a bug where the seoFilename and thumbnails were not correctly generated when using the ProductsController to manage images.
    c. It might be worth removing image management from the ProductsController and only supporting it through the ProductPicturesController.

  6. Modified ImageMappingDto to return ProductId in the ProductsController class.
    a. Previously you could not specify or retrieve the ProductId from the ImageMappingDto class.
    b. I have added a ProductId property which allows for the use of this class in the ProductPicturesController.

  7. Modified CategoriesController to correctly register the Url slug for a category.
    a. Previously if you created or updated a Category and did not provide a SeName the creation process was not successfully completed.
    b. This can be replicated by creating or updating a category and ommitting the SeName property,and then attempting to browse to the given category. The links created will not work because the slug for the category is not created.
    c. There might need to be some special case for this problem because of the way that the API is supposed to ignore null properties but it seems to me that the default behaviour is broken.

Also merged previous pull request #181

Added SinceId for OrderCount in OrdersController
Added SinceId for OrderCount in OrdersController
…nd Products

Modified JsonModelBinder, OrdersController, ImageDto, DeleteProduct, ImageMappingDto and CategoriesController. Added ProductPicturesController.
@MaximG1234 MaximG1234 changed the title List of changes in pull request. Fixes changes to JsonModelBinder, OrdersController, Images and Categories. Jan 24, 2019
@MaximG1234 MaximG1234 changed the title Fixes changes to JsonModelBinder, OrdersController, Images and Categories. Fixes and changes to JsonModelBinder, OrdersController, Images and Categories. Jan 24, 2019
@MaximG1234
Copy link
Contributor Author

I am happy to discuss all these changes in further detail with the owners of this project. Would really appreciate if the owners provided some feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant