-
Notifications
You must be signed in to change notification settings - Fork 149
Add GraphQL API to update the mutated short links #239
Comments
I can take on this issue, and update everyone this Friday regarding the progress made. |
Required Steps
|
GraphQL Schema type AuthMutation {
createURL(url: URLInput!, isPublic: Boolean!): URL
updateURL(alias: String!, url: URLInput!): URL
deleteURL(alias: String!)
} |
@oatovar Looks great to me! For your information, we may consider combining all operations on urls to a single place. Maybe still keep the separate interfaces. Not sure what to do yet. The goal is to reduce unnecessary complexities. |
@byliuyang Definitely understandable, making this flexible enough for future additions takes time. |
@byliuyang Should the user have the ability to mutate the short alias as well as the long link? |
@oatovar I think so. |
@oatovar It's seems that Facebook officially used only one input type: https://graphql.org/graphql-js/mutations-and-input-types/ |
@byliuyang Thank you for the referenced example! In the example, the application doesn't verify if the content and author is ommited since the arugments are optional. In Short's case, the original url is always required during creation, so having one input where it's both optional for both creation and mutation would be misleading when using the schema. We could have one single input type where all fields are optional and enforce requirements in the implemenation, but it might lead to confusion down the road. So far these are the two solutions with their tradeoffs:
After going over the tradeoffs, I think that one input might be the better option at this time. The chances of causing confusion will be very low if the error handling explicitly describe failure reasons as it is now. However, I am open to either option and the changes required to update the current work on this feature will be very little regardless of the option taken. What are your thoughts? |
@oatovar It's a hard a decision. We can go with 1 input for now and look back in the future. |
From #237
The text was updated successfully, but these errors were encountered: