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(GraphQL): Rename objectId to id #5985

Merged

Conversation

douglasmuraoka
Copy link
Contributor

Renames objectId to id for the GraphQL API. Queries, mutations, custom and generic types were updated.
Removes RELATION_INPUT and POINTER_INPUT. Now the user just needs to provide the ID of the object to link.

Renames `objectId` to `id` for the GraphQL API. Queries, mutations,
custom and generic types were updated.
Removes `RELATION_INPUT` and `POINTER_INPUT`. Now the user just need
to provide the ID of the object to link.
@Moumouls
Copy link
Member

Currently

object.id = object.objectId
delete object.objectId

I think this is not a good solution, we will have many problems in the future.

Here to main goal is only to map id as objectId, i think the best way is to use a resolver on id field. For inputs, we just need to adjust constraints transformer and some hard coded objectId on inputs.

It will be much easier to maintain and easy to use for future features.

Example: not tested

Change Interface

const CREATE_RESULT_FIELDS = {
  id: OBJECT_ID_ATT,
  createdAt: CREATED_AT_ATT,
};

Output: parseClassTypes -> outputFields(), map the return classic objectId to id

const type = mapOutputType(
  parseClass.fields[field].type,
  parseClass.fields[field].targetClass,
  parseGraphQLSchema.parseClassTypes
);
if(field === "objectId"){
  return {
    ...fields,
    'id': {
      description: 'The ID (same as the old Parse objectId)',
      type: new GraphQLNonNull(GraphQLID),
      resolve: (parent) => parent.objectId
    },
  };
}
...

@davimacedo what do you think ?

@codecov
Copy link

codecov bot commented Aug 28, 2019

Codecov Report

Merging #5985 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5985      +/-   ##
==========================================
+ Coverage   93.69%   93.75%   +0.06%     
==========================================
  Files         156      156              
  Lines       10926    10939      +13     
==========================================
+ Hits        10237    10256      +19     
+ Misses        689      683       -6
Impacted Files Coverage Δ
src/GraphQL/transformers/query.js 46.87% <ø> (ø) ⬆️
src/GraphQL/loaders/parseClassQueries.js 97.56% <100%> (ø) ⬆️
src/GraphQL/loaders/objectsQueries.js 97.33% <100%> (+0.31%) ⬆️
src/GraphQL/loaders/objectsMutations.js 97.05% <100%> (+0.08%) ⬆️
src/GraphQL/transformers/mutation.js 96.36% <100%> (ø) ⬆️
src/GraphQL/parseGraphQLUtils.js 93.54% <100%> (+0.69%) ⬆️
src/GraphQL/loaders/parseClassMutations.js 98.57% <100%> (ø) ⬆️
src/GraphQL/loaders/parseClassTypes.js 85.71% <100%> (+0.12%) ⬆️
src/GraphQL/loaders/defaultGraphQLTypes.js 97.2% <100%> (-0.02%) ⬇️
src/Routers/UsersRouter.js 94.19% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8d8cd6...c23c079. Read the comment docs.

@douglasmuraoka
Copy link
Contributor Author

@Moumouls I've undone the objectId attribute deletion for Parse class queries and mutations. Now it affects only the generic queries, which I don't think it's a major issue since it returns objects.
For the id, since all Parse output types have the same objectId type, I've changed the OBJECT_ID_ATT type to resolve source.objectId. When used for input, it resolves to source.id, which is the case where we wouldn't need to do any treatment.

@davimacedo any thoughts?

Copy link
Member

@Moumouls Moumouls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. I have just one additional question.

src/GraphQL/loaders/defaultGraphQLTypes.js Outdated Show resolved Hide resolved
@davimacedo davimacedo merged commit b47d9fb into parse-community:master Aug 30, 2019
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* refactor(GraphQL): Rename objectId to id

Renames `objectId` to `id` for the GraphQL API. Queries, mutations,
custom and generic types were updated.
Removes `RELATION_INPUT` and `POINTER_INPUT`. Now the user just need
to provide the ID of the object to link.

* fix: Column "id" not found on Postgres

* fix: Avoid deleting Parse class objectId

* fix: Undo objectId removal on mutations

* fix: Handle generic mutation id
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.

3 participants