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

Unify field naming related to publication date #3479

Closed
maarcingebala opened this issue Dec 18, 2018 · 2 comments · Fixed by #3706
Closed

Unify field naming related to publication date #3479

maarcingebala opened this issue Dec 18, 2018 · 2 comments · Fixed by #3706
Assignees
Labels
graphql Issues related to the GraphQL API

Comments

@maarcingebala
Copy link
Member

maarcingebala commented Dec 18, 2018

There is naming inconsistency in the fields related to publication date and availability:
- Product.available_on + Product.is_published
- Page.available_on + Page.is_visible
- Collection.published_date + Collection.is_published (will be merged soon, #3369)

Also fields is_visible/is_published are problematic since they can be set to True but if the future publication date is set, an item won't be available publicly.

What I would propose to do about it:

  • introduce publication_date = models.DateTimeField() field in all of the above-mentioned models (currently we have DateField but I think some may want to specify exact time as well)
  • rename is_published/is_visible fields to is_draft - I think that what those fields do is telling if as an admin I've already finished editing an object or I still have something to add, but I'd like to hear some feedback about this one. Let's consistently use is_published for this kind of fields.
@patrys
Copy link
Member

patrys commented Dec 18, 2018

Why I like is_published over is_draft is the way it looks when used in the API/client:

query Q {
  products(isPublished: true) {
    edges {
      node {
        ...
      }
    }
  }
}

@maarcingebala
Copy link
Member Author

We could have isPublished in the API and also as a helper method in the model, but it should be a property taking into account publication_date and is_draft underneath.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants