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

Refactor Images handling in API #2813

Closed
4 tasks
artursmet opened this issue Sep 10, 2018 · 2 comments
Closed
4 tasks

Refactor Images handling in API #2813

artursmet opened this issue Sep 10, 2018 · 2 comments
Labels
graphql Issues related to the GraphQL API technical debt

Comments

@artursmet
Copy link
Contributor

Now we have separated types for ProductImage and a standalone Image. This situation is caused by a need for storing more information for product images, like their reference to a particular variant and so on.

I recommend to change approach a bit - let's create an independent model for Image and replace all standalone versatileImageField fields with a OneToOneField which will refer to Image instance.

Above approach will allow us to unify all image endpoints in the API, also it may create a way to replace underlying thumbnailing library easier than now.

By the way of the refactoring we may also introduce more fields in the Image type, for example:

query {
  product(id: foo) {
    images {
      url(size: 100),
      width,
      height
    }
  }
}

Having extra fields for width and height will allow better integration with some native SDKs like Android, where exact image dimensions are required before rendering it. Size used in parameter is only one dimension - the backend decides which thumbnail will fit the best for the queried size.

Definition of done:

  • Create a model for Image
  • Replace all standalone versatileImageField fields with a relation to Image
  • Provide data migration mechanism
  • Refactor all API endpoints to follow new approach (products, collection, category etc)
@artursmet artursmet added technical debt graphql Issues related to the GraphQL API labels Sep 10, 2018
@maarcingebala maarcingebala added this to the GraphQL milestone Dec 12, 2018
@maarcingebala
Copy link
Member

We've unified the API a bit with PR #3429, namely, now all images are either of Image or ProductImage type. Before that PR, there were e.g. thumbnailUrl: String fields. Fields like background_image in Category or Collection type would still need to be refactored to model instances, but I'm still not convinced that this is the best approach.

@maarcingebala maarcingebala removed this from the GraphQL milestone Jan 31, 2019
@maarcingebala
Copy link
Member

Closing this since image types have been sorted out and use a common Image type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graphql Issues related to the GraphQL API technical debt
Projects
None yet
Development

No branches or pull requests

2 participants