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

Web API support #20

Closed
mariannk opened this issue Jan 14, 2016 · 9 comments
Closed

Web API support #20

mariannk opened this issue Jan 14, 2016 · 9 comments

Comments

@mariannk
Copy link

mariannk commented Jan 14, 2016

Most serious e-commerce businesses doesn't use ecommerce solution alone. They also use a decent accounting, crm, inventory, order management, support and operations management solutions which needs to work very closely with the ecommerce solution. However, adding all those features to the nopCommerce is out of the scope definitely... but giving a decent set of API for integration will be very helpful for all service providers and retailers using nopCommerce.

This will greatly enhance adoption of nopCommerce for the serious retailers as most of them will use it. Moreover, providing it also enables solution providers to come up with generic plugins for example, integration for salesforce or vtiger crm, integration for quickoffice or openerp or any other solution!

See http://www.nopcommerce.com/boards/t/31212/nopcommerce-350-roadmap-and-estimated-release-date-lets-discuss.aspx#127120

UPDATE: nop-templates.com are going to contribute some great implementation soon
https://github.com/SevenSpikes/nopCommerce/tree/Web-Api-3.70
and
https://github.com/SevenSpikes/nopCommerce-Api-SampleApplication

RESULT IS AVAILABLE AT http://www.nopcommerce.com/p/2464/api-plugin.aspx

@suddenelfilio
Copy link

Nop-templates Web Api plugin looks ok, but I attended the devdays this year and spotted various culprits in their implementation:

  • Specifiying fields to be returned without transparantly querying them down to the database is no advantage => you still query all the data, but only at json serialization you filter out the data specified in fields... For what? better bandwidth? Bandwidth is cheap you should focus on server resources.
  • The partial updates using Delta are breaking convention. On one hand you can update only the fields you specify in a PUT, but on the other hand you delete all not mentioned items in a collection/relation? That's not consistent with the expectation of Partial updates.
  • Returning images as BASE64 is effectivly killing off any CDN browser/proxy/server caching you might use as you are still returning a bunch of string data. It would be better to provide the url to image and let the framework used figure out how to render it. Or at least provide a way to select the behavior.
  • Also no fan of snake casing as it does not add any value to the REST principle. If you want to access items from an order your should use api/order//items and not api/order//order_items => why specify order_items when the parent entity you are querying though is already an Order. Logic provides that the items will be order items. It is the same as saying I have an Order class with properties like OrderId, OrderDate, OrderItems and so on... No those properties should be Id, Date, Items really no need for repeating/prefixen the "Order".
    -Not sure about this one, but I really hope that things like sorting/filtering/paging are done on the datasets in memory (like the fields "feature") because again you just lose any performance gain your might get by propagating this down to the data query itself.

Okay gonna stop rambling on now... sorry for that, but I'm an REST API purist when comes to stuff like this.

Another note I believe you might benefit more from introducing something like Web Hooks instead of exposing the API.

@AndreiMaz
Copy link
Member

@suddenelfilio Thanks a lot for your feedback. Could you please copy and post it at https://github.com/SevenSpikes/nopCommerce/issues. This way Boyko will see it

@poyker
Copy link
Contributor

poyker commented Oct 26, 2016

@suddenelfilio

Thank you for your feedback!

Please see my comments below:

  1. Yes, the fields work on JSON serialization level but it doesn't mean they are useless. From an API consumer point of view if I use the fields parameter I would expect exactly this - reduce the size of the response.
    I agree with you that the implementation should try to get only the specified fields from the database but I have to admit that we couldn't find a feasible solution. If you have done this for the APIs that you have implemented and you are willing to share your experience with us then I will be more than happy to implement it into the API plugin.
  2. The partial updates actually should be PATCH rather than PUT if we are to follow the conventions explicitly. If you are referring to the product images maybe you have not understood how exactly they work.
    For example if you want to update a product that has some images and you don't specify the Images property then they will NOT be deleted.

    PUT /api/products/5 { "product" : { "name" : "updated name" } }

In the request above only the product name will be updated and nothing will be deleted.

If you want to delete the product images then you need to explicitly specify the Images property like this:

PUT /api/products/5
{
"product" : {
"images" : []
}
}

The request above will delete all product images for this product.

Actually to fully manage the product images there should be separate endpoints i.e (/api/products/{#id}/images) but they are still not implemented.

3. Agree. There should be a setting in the API plugin that will control if the returned images should be in Base64 format or just a link to the image i.e "attachment" or "src". I will create an issue for this.
4. As I said on the conference - the API design is a never ending debate :) So we better choose one naming convention and stick with it. I think the majority of APIs use snake_case and that is why we decided to use this one. Regarding the OrderItems, I agree with you. It could be simply Items. We did this mechanically because we have a separate controller called OrderItemsController and also OrderItemDTO etc. and that is why we made the routes with OrderItems. I will create a new issue to correct this.

Regarding the paging/filtering/sorting, it is all done on the DB level.

To be honest the Customers is the only area that I really don't like how we have implemented. But if you have a chance to take a look at the code and give us feedback will be much appreciated.

WebHooks - this is really what the next step for the API plugin should be. Let's first make sure the rest of the things are stable and working and we will do this one as well. I will create an issue for this.

Thank you again!

@alijunkison
Copy link

@AndreiMaz and @poyker,

My team works with nopCommerce and we're very keen to begin using the API for integration projects, so I have three questions that I'm hoping you can answer please:

  1. Is the API version for nopCommerce 3.80 a beta release, or a full release of the API?
  2. What additional nopCommerce objects will you be supporting for the API in nopCommerce 3.90 (e.g. prices, promotions & discounts, loyalty points). Apologies if any that I've listed here are already included, but I couldn't find any information on them at this link: https://github.com/SevenSpikes/api-plugin-for-nopcommerce
  3. Will any new API functionality released for nop 3.90 be backwards compatible with nop 3.80?

I need to make a decision on whether to begin working with the API quite quickly, so would really appreciate your feedback.

Many thanks,

Ali

@alijunkison
Copy link

@AndreiMaz, I see this work item has just been closed, please can you provide an update on the status of the API?

Many thanks,

Ali

@AndreiMaz
Copy link
Member

@alijunkison
Copy link

Great - thanks @AndreiMaz!

Please can you confirm the answers to questions 2 & 3 in my above post if possible?

@AndreiMaz
Copy link
Member

@alijunkison please address these questions to "poyker" (original developer of this great plugin)!

@BabuBahir
Copy link

"Most serious e-commerce businesses doesn't use ecommerce solution alone. They also use a decent crm"... omg exactly the phrase I was looking for <3 (y)

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

No branches or pull requests

6 participants