From aa7208909971ced421250df966c8a3d69e98d35a Mon Sep 17 00:00:00 2001 From: Robert Zhu Date: Thu, 21 Apr 2016 16:15:45 -0400 Subject: [PATCH 1/2] Improve validation error message when field names conflict --- src/validation/rules/OverlappingFieldsCanBeMerged.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/validation/rules/OverlappingFieldsCanBeMerged.js b/src/validation/rules/OverlappingFieldsCanBeMerged.js index 2c2c8edcab..a773229b4d 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMerged.js +++ b/src/validation/rules/OverlappingFieldsCanBeMerged.js @@ -39,14 +39,16 @@ export function fieldsConflictMessage( responseName: string, reason: ConflictReasonMessage ): string { - return `Fields "${responseName}" conflict because ${reasonMessage(reason)}.`; + return `Fields "${responseName}" conflict because ${reasonMessage(reason)}` + + '. Use aliases on the fields to fetch both if this was intentional.'; } function reasonMessage(reason: ConflictReasonMessage): string { if (Array.isArray(reason)) { return reason.map(([ responseName, subreason ]) => `subfields "${responseName}" conflict because ${reasonMessage(subreason)}` - ).join(' and '); + ).join(' and ') + + '. Use aliases on the fields to fetch both if this was intentional.'; } return reason; } From 7d2f1916f110d985a8b4ae6ae8c6389f0cd09963 Mon Sep 17 00:00:00 2001 From: Robert Zhu Date: Mon, 25 Apr 2016 17:05:57 -0400 Subject: [PATCH 2/2] Remove extra hint and add unit test --- package.json | 1 + .../__tests__/OverlappingFieldsCanBeMerged-test.js | 13 +++++++++++++ .../rules/OverlappingFieldsCanBeMerged.js | 6 +++--- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index c8d24cee72..b8f5c03e4b 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ "babel-plugin-transform-runtime": "6.6.0", "babel-preset-es2015": "6.6.0", "chai": "3.5.0", + "chai-string": "1.2.0", "chai-subset": "1.2.2", "coveralls": "2.11.9", "eslint": "2.7.0", diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js b/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js index 457a0dad05..65e329601e 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js +++ b/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js @@ -7,6 +7,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ +import { expect } from 'chai'; import { describe, it } from 'mocha'; import { expectPassesRule, @@ -28,7 +29,10 @@ import { GraphQLString, GraphQLID, } from '../../type'; +import chai from 'chai'; +import chaiString from 'chai-string'; +chai.use(chaiString); describe('Validate: Overlapping fields can be merged', () => { @@ -753,6 +757,15 @@ describe('Validate: Overlapping fields can be merged', () => { `); }); + it('error message contains hint for alias conflict', () => { + // The error template should end with a hint for the user to try using + // different aliases. + const error = fieldsConflictMessage('x', 'a and b are different fields'); + const hint = 'Use different aliases on the fields to fetch both ' + + 'if this was intentional.'; + expect(error).to.endsWith(hint); + }); + }); }); diff --git a/src/validation/rules/OverlappingFieldsCanBeMerged.js b/src/validation/rules/OverlappingFieldsCanBeMerged.js index a773229b4d..38c506ddca 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMerged.js +++ b/src/validation/rules/OverlappingFieldsCanBeMerged.js @@ -40,15 +40,15 @@ export function fieldsConflictMessage( reason: ConflictReasonMessage ): string { return `Fields "${responseName}" conflict because ${reasonMessage(reason)}` + - '. Use aliases on the fields to fetch both if this was intentional.'; + '. Use different aliases on the fields to fetch both if this was ' + + 'intentional.'; } function reasonMessage(reason: ConflictReasonMessage): string { if (Array.isArray(reason)) { return reason.map(([ responseName, subreason ]) => `subfields "${responseName}" conflict because ${reasonMessage(subreason)}` - ).join(' and ') + - '. Use aliases on the fields to fetch both if this was intentional.'; + ).join(' and '); } return reason; }