From 491b5b0e0c4af04f6cfb4b228e7dcf8bb8636bc9 Mon Sep 17 00:00:00 2001 From: SamGodwin2 Date: Mon, 5 Aug 2019 12:44:25 +0100 Subject: [PATCH] Fixed a bug due to adding and deleting pages and sections. --- eq-author-api/schema/resolvers/base.js | 2 +- eq-author-api/schema/typeDefs.js | 2 +- eq-author/package.json | 6 +-- .../Design/MovePageModal/MovePageQuery.js | 6 ++- .../MoveSectionModal/MoveSectionQuery.js | 1 + .../App/section/Design/withDeleteSection.js | 39 ++++------------- .../section/Design/withDeleteSection.test.js | 42 +++++-------------- eq-author/src/graphql/deleteSection.graphql | 8 ++-- eq-author/yarn.lock | 9 ++-- 9 files changed, 37 insertions(+), 78 deletions(-) diff --git a/eq-author-api/schema/resolvers/base.js b/eq-author-api/schema/resolvers/base.js index d78e2e3aa9..d6c2a73298 100644 --- a/eq-author-api/schema/resolvers/base.js +++ b/eq-author-api/schema/resolvers/base.js @@ -228,7 +228,7 @@ const Resolvers = { const section = find(ctx.questionnaire.sections, { id: input.id }); remove(ctx.questionnaire.sections, section); onPageDeleted(ctx, input.id); - return section; + return ctx.questionnaire; }), moveSection: createMutation((_, { input }, ctx) => { const removedSection = first( diff --git a/eq-author-api/schema/typeDefs.js b/eq-author-api/schema/typeDefs.js index 8d0ac2e49e..b82479e4a4 100644 --- a/eq-author-api/schema/typeDefs.js +++ b/eq-author-api/schema/typeDefs.js @@ -554,7 +554,7 @@ type Mutation { duplicateQuestionnaire(input: DuplicateQuestionnaireInput!): Questionnaire createSection(input: CreateSectionInput!): Section updateSection(input: UpdateSectionInput!): Section - deleteSection(input: DeleteSectionInput!): Section + deleteSection(input: DeleteSectionInput!): Questionnaire moveSection(input: MoveSectionInput!): Section duplicateSection(input: DuplicateSectionInput!): Section updatePage(input: UpdatePageInput!): Page diff --git a/eq-author/package.json b/eq-author/package.json index a8a662432c..a77e972613 100644 --- a/eq-author/package.json +++ b/eq-author/package.json @@ -73,10 +73,10 @@ "svgo-loader": "latest", "terser-webpack-plugin": "latest", "url-loader": "latest", + "wait-for-expect": "latest", "webpack": "latest", "webpack-dev-server": "latest", - "webpack-manifest-plugin": "latest", - "wait-for-expect": "latest" + "webpack-manifest-plugin": "latest" }, "dependencies": { "@sentry/browser": "latest", @@ -104,7 +104,7 @@ "lodash": "latest", "polished": "latest", "react": "latest", - "react-apollo": "latest", + "react-apollo": "^2.5.8", "react-dom": "latest", "react-firebaseui": "latest", "react-hot-loader": "latest", diff --git a/eq-author/src/App/page/Design/MovePageModal/MovePageQuery.js b/eq-author/src/App/page/Design/MovePageModal/MovePageQuery.js index 5a0838c542..9301dda772 100644 --- a/eq-author/src/App/page/Design/MovePageModal/MovePageQuery.js +++ b/eq-author/src/App/page/Design/MovePageModal/MovePageQuery.js @@ -4,7 +4,11 @@ import query from "graphql/getQuestionnaire.graphql"; import PropTypes from "prop-types"; const MovePageQuery = ({ questionnaireId, children }) => ( - + {({ loading, data }) => children({ loading, data })} ); diff --git a/eq-author/src/App/section/Design/SectionEditor/MoveSectionModal/MoveSectionQuery.js b/eq-author/src/App/section/Design/SectionEditor/MoveSectionModal/MoveSectionQuery.js index 108648ba9d..9bb6fe8713 100644 --- a/eq-author/src/App/section/Design/SectionEditor/MoveSectionModal/MoveSectionQuery.js +++ b/eq-author/src/App/section/Design/SectionEditor/MoveSectionModal/MoveSectionQuery.js @@ -6,6 +6,7 @@ import PropTypes from "prop-types"; const MoveSectionQuery = ({ questionnaireId, children }) => ( { export const handleDeletion = ( { history, onAddSection, match: { params } }, - questionnaire + { data }, + oldQuestionnaire ) => { + const questionnaire = data.deleteSection; const { sectionId, questionnaireId } = params; - if (questionnaire.sections.length === 1) { + if (questionnaire.sections.length === 0) { return onAddSection(); } - const nextSection = getNextSection(questionnaire.sections, sectionId); + const nextSection = getNextSection(oldQuestionnaire.sections, sectionId); const nextPage = nextSection.pages[0]; history.push( @@ -47,30 +48,6 @@ export const handleDeletion = ( ); }; -export const deleteUpdater = (questionnaireId, sectionId) => ( - proxy, - result -) => { - const id = `Questionnaire${questionnaireId}`; - const questionnaire = proxy.readFragment({ id, fragment }); - - remove(questionnaire.sections, { id: sectionId }); - - const sections = questionnaire.sections.map(section => ({ - ...section, - questionnaire: result.data.deleteSection.questionnaire, - })); - - proxy.writeFragment({ - id, - fragment, - data: { - ...questionnaire, - sections, - }, - }); -}; - export const displayToast = (ownProps, questionnaire) => { const { match: { params }, @@ -94,7 +71,6 @@ export const mapMutateToProps = ({ ownProps, mutate }) => ({ client, } = ownProps; const section = { id: sectionId }; - const update = deleteUpdater(params.questionnaireId, sectionId); const questionnaire = client.readFragment({ id: `Questionnaire${params.questionnaireId}`, @@ -103,11 +79,10 @@ export const mapMutateToProps = ({ ownProps, mutate }) => ({ const mutation = mutate({ variables: { input: section }, - update, }); return mutation - .then(() => handleDeletion(ownProps, questionnaire)) + .then(data => handleDeletion(ownProps, data, questionnaire)) .then(() => displayToast(ownProps, questionnaire)) .then(() => mutation); }, diff --git a/eq-author/src/App/section/Design/withDeleteSection.test.js b/eq-author/src/App/section/Design/withDeleteSection.test.js index 339a663d8a..25cb927aa8 100644 --- a/eq-author/src/App/section/Design/withDeleteSection.test.js +++ b/eq-author/src/App/section/Design/withDeleteSection.test.js @@ -1,9 +1,4 @@ -import { - mapMutateToProps, - deleteUpdater, - handleDeletion, -} from "./withDeleteSection"; -import fragment from "graphql/questionnaireFragment.graphql"; +import { mapMutateToProps, handleDeletion } from "./withDeleteSection"; describe("withDeleteSection", () => { let history, mutate, result, ownProps, onAddSection, showToast; @@ -43,7 +38,10 @@ describe("withDeleteSection", () => { result = { data: { - deleteSection: deletedPage, + deleteSection: { + id: "questionnaire", + sections: [], + }, }, }; @@ -70,25 +68,6 @@ describe("withDeleteSection", () => { mutate = jest.fn(() => Promise.resolve(result)); }); - describe("deleteUpdater", () => { - it("should remove the section from the cache", () => { - const id = `Questionnaire${questionnaire.id}`; - const readFragment = jest.fn(() => questionnaire); - const writeFragment = jest.fn(); - - const updater = deleteUpdater(questionnaire.id, currentSection.id); - updater({ readFragment, writeFragment }, result); - - expect(readFragment).toHaveBeenCalledWith({ id, fragment }); - expect(writeFragment).toHaveBeenCalledWith({ - id, - fragment, - data: questionnaire, - }); - expect(questionnaire.sections).not.toContain(currentSection); - }); - }); - describe("mapMutateToProps", () => { let props; @@ -152,19 +131,18 @@ describe("withDeleteSection", () => { describe("handleDeletion", () => { describe("when only one section in questionnaire", () => { - beforeEach(() => { - questionnaire.sections = [currentSection]; - }); - it("should add new section", () => { - handleDeletion(ownProps, questionnaire); + handleDeletion(ownProps, result, {}); expect(onAddSection).toHaveBeenCalled(); }); }); describe("when more than one section in questionnaire", () => { it("should redirect to another section", () => { - handleDeletion(ownProps, questionnaire); + result.data.deleteSection.sections = [ + { id: "section 1", pages: [{ id: "page 1" }] }, + ]; + handleDeletion(ownProps, result, questionnaire); expect(history.push).toHaveBeenCalled(); }); }); diff --git a/eq-author/src/graphql/deleteSection.graphql b/eq-author/src/graphql/deleteSection.graphql index 1a0f33b203..ec806b3bcb 100644 --- a/eq-author/src/graphql/deleteSection.graphql +++ b/eq-author/src/graphql/deleteSection.graphql @@ -1,11 +1,11 @@ mutation DeleteSection($input: DeleteSectionInput!) { deleteSection(input: $input) { id - questionnaire { + sections { id - questionnaireInfo { - totalSectionCount - } + } + questionnaireInfo { + totalSectionCount } } } diff --git a/eq-author/yarn.lock b/eq-author/yarn.lock index a931b4d6d1..30f6f8e196 100644 --- a/eq-author/yarn.lock +++ b/eq-author/yarn.lock @@ -9467,12 +9467,13 @@ rc@^1.2.7: minimist "^1.2.0" strip-json-comments "~2.0.1" -react-apollo@latest: - version "2.5.6" - resolved "https://registry.yarnpkg.com/react-apollo/-/react-apollo-2.5.6.tgz#98a59d0eea31432ed001e6a033e11a58139ffc31" - integrity sha512-WWX5UykTtmW6+awjqEsSWSdvVyZv/vsavUgpdI4ddn4CBdz47INC+iTdJBnYaUFMB24GmqjFFSoSd98gu1xqKA== +react-apollo@^2.5.8: + version "2.5.8" + resolved "https://registry.yarnpkg.com/react-apollo/-/react-apollo-2.5.8.tgz#c7a593b027efeefdd8399885e0ac6bec3b32623c" + integrity sha512-60yOQrnNosxU/tRbOxGDaYNLFcOKmQqxHPhxyvKTlGIaF/rRCXQRKixUgWVffpEupSHHD7psY5k5ZOuZsdsSGQ== dependencies: apollo-utilities "^1.3.0" + fast-json-stable-stringify "^2.0.0" hoist-non-react-statics "^3.3.0" lodash.isequal "^4.5.0" prop-types "^15.7.2"