diff --git a/.vscode/launch.json b/.vscode/launch.json index f2df090b26..5a117a8249 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -14,6 +14,15 @@ "console": "integratedTerminal", "program": "${workspaceFolder}/node_modules/.bin/jest", "args": ["${fileBasenameNoExtension}"] + }, + { + "name": "Debug functions-v2 test", + "request": "launch", + "type": "node", + "console": "integratedTerminal", + "program": "${workspaceFolder}/functions-v2/node_modules/.bin/jest", + "args": ["${fileBasenameNoExtension}"], + "cwd": "${workspaceFolder}/functions-v2" } ] } diff --git a/README.md b/README.md index 3f0cdb4579..608a1ffc13 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,6 @@ There are a number of URL parameters that can aid in testing: |`fakeClass` |string |Class id for demo, qa, or test modes.| |`fakeUser` |`(student\|teacher):`|Configure user type and (optionally) id.| |`qaGroup` |string |Group id for qa, e.g. automated tests.| -|`qaClear` |`all\|class\|offering` |Extent of database clearing for automated tests.| |`firebase` |`emulator\|` |Target emulator for firebase realtime database calls.| |`firestore` |`emulator\|` |Target emulator for firestore database calls.| |`functions` |`emulator\|` |Target emulator-hosted firebase functions.| @@ -183,11 +182,11 @@ The Standalone Document Editor also supports a `readOnly` url param. If you spec ### QA -Along with `dev`, `test`, `authed` and `demo` modes the app has a `qa` mode. QA mode uses the same parameters as demo mode with two additional parameters: +Along with `dev`, `test`, `authed` and `demo` modes the app has a `qa` mode. QA mode uses the same parameters as demo mode with one additional parameter: -1. qaGroup - the group to automatically assign the fake user to after connecting to the database. -2. qaClear - either "all", "class" or "offering". When this parameter is present the QA database is cleared at the level requested based on the user parameters. - This is useful to clear data between automated QA runs. When complete the app will display `QA Cleared: OK`. +qaGroup - the group to automatically assign the fake user to after connecting to the database. + +Additionally in `qa` mode the "root" in Firestore and the Realtime database is based on the Firebase user uid. This user is stored in session storage so each new tab will start a new root. In Cypress session storage is cleared between tests so each new test will have its own root. ### To run Cypress integration tests: - `npm run test:local` @@ -209,7 +208,6 @@ implementation simpler. Existing commands at the moment: - setupGroup - upLoadFile - - clearQAData ## License diff --git a/cypress/config/cypress.dev.json b/cypress/config/cypress.dev.json deleted file mode 100644 index 2aafc67ebb..0000000000 --- a/cypress/config/cypress.dev.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "baseUrl": "http://localhost:8080/" -} - diff --git a/cypress/config/cypress.local.json b/cypress/config/cypress.local.json index d36314102c..00ea18a126 100644 --- a/cypress/config/cypress.local.json +++ b/cypress/config/cypress.local.json @@ -1,4 +1,5 @@ { "baseUrl": "http://localhost:8080/", - "hideXHRInCommandLog": true + "hideXHRInCommandLog": true, + "defaultCommandTimeout": 5000 } diff --git a/cypress/e2e/cleanup/remove_teacher_comment_spec.js b/cypress/e2e/cleanup/remove_teacher_comment_spec.js index 376d81d925..df7970ca66 100644 --- a/cypress/e2e/cleanup/remove_teacher_comment_spec.js +++ b/cypress/e2e/cleanup/remove_teacher_comment_spec.js @@ -56,7 +56,6 @@ function beforePortalTest(url, clueTeacher, reportUrl) { function beforeTest() { const queryParams = `${Cypress.config("qaUnitTeacher6Network")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); cy.openTopTab("problems"); @@ -78,7 +77,7 @@ describe('Delete Teacher Comments In chat panel', () => { // Teacher 1 tile comment chatPanel.deleteTeacherComments(); }); - + cy.log("login teacher2 and setup clue chat"); cy.logout(portalUrl); beforePortalTest(portalUrl, clueTeacher2, reportUrl2); @@ -93,7 +92,7 @@ describe('Delete Teacher Comments In chat panel', () => { cy.clickProblemResourceTile(tab.sectionCode); // Teacher 2 tile comment chatPanel.deleteTeacherComments(); - }); + }); }); it('Delete chat panel comment tags', () => { beforeTest(); @@ -102,7 +101,7 @@ describe('Delete Teacher Comments In chat panel', () => { cy.openTopTab("problems"); cy.openProblemSection("Introduction"); chatPanel.deleteTeacherComments(); - + cy.log('Delete comment tags on tile comment'); cy.openTopTab("problems"); cy.clickProblemResourceTile('introduction'); diff --git a/cypress/e2e/functional/document_tests/bookmark_test_spec.js b/cypress/e2e/functional/document_tests/bookmark_test_spec.js index a59fa14f69..dc5d503eb2 100644 --- a/cypress/e2e/functional/document_tests/bookmark_test_spec.js +++ b/cypress/e2e/functional/document_tests/bookmark_test_spec.js @@ -12,7 +12,6 @@ const queryParams1 = `${Cypress.config("qaConfigSubtabsUnitStudent5")}`; const queryParams2 = `${Cypress.config("qaConfigSubtabsUnitTeacher1")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } @@ -76,7 +75,7 @@ context('Bookmarks', function () { // resourcesPanel.getCanvasStarIcon('class-work', 'workspaces', copyDocumentTitle).should('have.class', 'starred'); // cy.openSection('class-work', 'bookmarks'); // resourcesPanel.getCanvasItemTitle('class-work', 'bookmarks').contains(copyDocumentTitle).should('exist'); - }) + }); it('Test bookmarks for teacher', function () { beforeTest(queryParams1); let copyDocumentTitle = 'copy Investigation'; @@ -120,5 +119,5 @@ context('Bookmarks', function () { resourcesPanel.getCanvasStarIcon('class-work', 'workspaces', title).should('have.class', 'starred'); cy.openSection('class-work', 'bookmarks'); resourcesPanel.getCanvasItemTitle('class-work', 'bookmarks').contains(title).should('exist'); - }) -}) + }); +}); diff --git a/cypress/e2e/functional/document_tests/canvas_test_spec.js b/cypress/e2e/functional/document_tests/canvas_test_spec.js index d087b743ed..3d890665d2 100644 --- a/cypress/e2e/functional/document_tests/canvas_test_spec.js +++ b/cypress/e2e/functional/document_tests/canvas_test_spec.js @@ -28,7 +28,6 @@ const queryParams4 = "?appMode=demo&demoName=BrokenDocs&fakeClass=1&fakeUser=stu const title = "QA 1.1 Solving a Mystery with Proportional Reasoning"; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/copy_doc_test_spec.js b/cypress/e2e/functional/document_tests/copy_doc_test_spec.js index 244beeddb4..1d8dc319de 100644 --- a/cypress/e2e/functional/document_tests/copy_doc_test_spec.js +++ b/cypress/e2e/functional/document_tests/copy_doc_test_spec.js @@ -31,7 +31,6 @@ let canvas = new Canvas; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/exemplar_test_spec.js b/cypress/e2e/functional/document_tests/exemplar_test_spec.js index 37c7ce0239..8f0905498b 100644 --- a/cypress/e2e/functional/document_tests/exemplar_test_spec.js +++ b/cypress/e2e/functional/document_tests/exemplar_test_spec.js @@ -18,7 +18,6 @@ const exemplarName = "First Exemplar"; const exemplarInfo = "Ivan Idea: First Exemplar"; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/group_chooser_spec.js b/cypress/e2e/functional/document_tests/group_chooser_spec.js index 4c1895dd09..af0eee792c 100644 --- a/cypress/e2e/functional/document_tests/group_chooser_spec.js +++ b/cypress/e2e/functional/document_tests/group_chooser_spec.js @@ -20,10 +20,6 @@ const defaultSetupOptions = { problem }; -function beforeTest() { - cy.clearQAData('all'); -} - function setup(student, opts = {}) { const options = { ...defaultSetupOptions, ...opts }; cy.visit('/?appMode=qa&fakeClass=' + fakeClass + '&fakeUser=student:' + student + '&problem=' + options.problem + '&unit=./demo/units/qa/content.json'); @@ -40,8 +36,6 @@ function setup(student, opts = {}) { context('Test student join a group', function () { it('Test student join a group', function () { - beforeTest(); - cy.log('Student 1 will join and will verify Join Group Dialog comes up with welcome message to correct student'); setup(student1); cy.get('.app > .join > .join-title').should('contain', 'Join Group'); diff --git a/cypress/e2e/functional/document_tests/group_test_readonly_spec.js b/cypress/e2e/functional/document_tests/group_test_readonly_spec.js index e312142c03..f6f3548f6d 100644 --- a/cypress/e2e/functional/document_tests/group_test_readonly_spec.js +++ b/cypress/e2e/functional/document_tests/group_test_readonly_spec.js @@ -25,7 +25,6 @@ context('Test group functionalities', function () { it('4-up view read-only', function () { cy.log('students to check each others tiles in 4-up view read-only'); - cy.clearQAData('all'); setupTest(0); setupTest(1); diff --git a/cypress/e2e/functional/document_tests/group_test_spec.js b/cypress/e2e/functional/document_tests/group_test_spec.js index 6d87fdbcf4..273721a135 100644 --- a/cypress/e2e/functional/document_tests/group_test_spec.js +++ b/cypress/e2e/functional/document_tests/group_test_spec.js @@ -45,7 +45,6 @@ context('Test group functionalities', function () { it('4-up view tests', function () { cy.log('will set up groups'); - cy.clearQAData('all'); setupTest(0); setupTest(1); setupTest(2); diff --git a/cypress/e2e/functional/document_tests/header_test_spec.js b/cypress/e2e/functional/document_tests/header_test_spec.js index 1f5832bf39..2805f4703a 100644 --- a/cypress/e2e/functional/document_tests/header_test_spec.js +++ b/cypress/e2e/functional/document_tests/header_test_spec.js @@ -4,7 +4,6 @@ const header = new Header; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/nav_panel_test_spec.js b/cypress/e2e/functional/document_tests/nav_panel_test_spec.js index 58d3da527b..2084cbfcfa 100644 --- a/cypress/e2e/functional/document_tests/nav_panel_test_spec.js +++ b/cypress/e2e/functional/document_tests/nav_panel_test_spec.js @@ -17,7 +17,6 @@ const queryParams4 = `${Cypress.config("qaNoSectionProblemTabUnitStudent5")}`; const queryParams5 = `${Cypress.config("qaConfigSubtabsUnitStudent5")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js b/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js index add46e38f4..b13e982803 100644 --- a/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js +++ b/cypress/e2e/functional/document_tests/student_teacher_4up_readonly_spec.js @@ -144,8 +144,6 @@ function setupTestBrain(studentIndex) { context('Test 4-up and 1-up views tiles read only functionalities', function () { it('4-up and 1-up views read-only text, table, geometry, drawing, expression, numberline, image, datacard tiles', function () { - cy.clearQAData('all'); - setupTest(0); setupTest(1); @@ -174,8 +172,6 @@ context('Test 4-up and 1-up views tiles read only functionalities', function () }); it('4-up and 1-up views read-only dataflow, expression, xy plot tiles', function () { - cy.clearQAData('all'); - setupTestBrain(0); setupTestBrain(1); diff --git a/cypress/e2e/functional/document_tests/student_test_spec.js b/cypress/e2e/functional/document_tests/student_test_spec.js index b2f3b16a35..e600f687f9 100644 --- a/cypress/e2e/functional/document_tests/student_test_spec.js +++ b/cypress/e2e/functional/document_tests/student_test_spec.js @@ -12,7 +12,6 @@ let student = '5', group = '5'; function beforeTest(queryParams) { - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js b/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js index 58207dbdf5..30bf1269f5 100644 --- a/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js +++ b/cypress/e2e/functional/document_tests/tiles_copy_test_spec.js @@ -54,7 +54,6 @@ const tiles2 = [ ]; function beforeTest(queryParams) { - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/document_tests/workspace_test_spec.js b/cypress/e2e/functional/document_tests/workspace_test_spec.js index 48d62d6b10..00cf920104 100644 --- a/cypress/e2e/functional/document_tests/workspace_test_spec.js +++ b/cypress/e2e/functional/document_tests/workspace_test_spec.js @@ -6,7 +6,6 @@ let clueCanvas = new ClueCanvas, function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/teacher_tests/history_playback_spec.js b/cypress/e2e/functional/teacher_tests/history_playback_spec.js index 141011d9e3..bd32d99cdc 100644 --- a/cypress/e2e/functional/teacher_tests/history_playback_spec.js +++ b/cypress/e2e/functional/teacher_tests/history_playback_spec.js @@ -17,7 +17,6 @@ function moveSliderTo(percent) { } function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); clueCanvas.getInvestigationCanvasTitle().text().then((investigationTitle) => { diff --git a/cypress/e2e/functional/teacher_tests/teacher_chat_spec.js b/cypress/e2e/functional/teacher_tests/teacher_chat_spec.js index ef92289a7d..43ebd7c8a2 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_chat_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_chat_spec.js @@ -34,7 +34,6 @@ const ss = [{ "section": "problems", const comment = [ "This is document comment for ", "This is tile comment for " ]; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); cy.openTopTab("problems"); @@ -44,7 +43,6 @@ function beforeTest(params) { } function beforeTestCommentedDocumentList(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); cy.openTopTab("my-work"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_curation_spec.js b/cypress/e2e/functional/teacher_tests/teacher_curation_spec.js index 91b4191038..169d047392 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_curation_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_curation_spec.js @@ -12,7 +12,6 @@ const defaultProblemDocTitle = "QA 1.1 Solving a Mystery with Proportional Reaso function beforeTest() { const queryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_dashboard_4_quadrants_spec.js b/cypress/e2e/functional/teacher_tests/teacher_dashboard_4_quadrants_spec.js index 3b76e2aef3..ce4b3cdbc8 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_dashboard_4_quadrants_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_dashboard_4_quadrants_spec.js @@ -4,7 +4,6 @@ let dashboard = new TeacherDashboard(); function beforeTest() { const queryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); dashboard.switchView("Dashboard"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_dashboard_spec.js b/cypress/e2e/functional/teacher_tests/teacher_dashboard_spec.js index 21087d44f1..97a8f90d2f 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_dashboard_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_dashboard_spec.js @@ -6,7 +6,6 @@ let clueCanvas = new ClueCanvas; function beforeTest() { const queryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); dashboard.switchView("Dashboard"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_network_spec.js b/cypress/e2e/functional/teacher_tests/teacher_network_spec.js index 43df7a03f3..a580b7c2b9 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_network_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_network_spec.js @@ -8,7 +8,6 @@ let teacherNetwork = new TeacherNetwork; function beforeTest() { const queryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_share_spec.js b/cypress/e2e/functional/teacher_tests/teacher_share_spec.js index a698e22df1..1060f74a63 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_share_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_share_spec.js @@ -8,7 +8,6 @@ const teacherQueryParams = `${Cypress.config("qaConfigSubtabsUnitTeacher1")}`; const studentQueryParams = `${Cypress.config("qaConfigSubtabsUnitStudent5")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); @@ -23,48 +22,43 @@ function verifySwitch(publicOrPrivate) { function verifyStudentSeesAsPrivate() { cy.get('.tab-sort-work').click(); cy.get('.section-header-arrow').click({multiple: true}); - cy.get('.thumbnail-private').should('exist'); + cy.contains('[data-test="sort-work-list-items"]','Teacher 1:') + .should('have.descendants', '.thumbnail-private'); } function verifyStudentSeesAsPublic() { cy.get('.tab-sort-work').click(); cy.get('.section-header-arrow').click({multiple: true}); - cy.get('.thumbnail-public').should('not.exist'); + cy.contains('[data-test="sort-work-list-items"]','Teacher 1:') + .should('not.have.descendants', '.thumbnail-private'); } context('Teacher Sharing', function() { - describe('verify share functionality', function() { - it('loads teacher document as private', function() { - beforeTest(teacherQueryParams); - verifySwitch('private'); - }); + it('verify share functionality', function() { + cy.log('loads teacher document as private'); + beforeTest(teacherQueryParams); + verifySwitch('private'); - // TODO: Reinstate the tests below when all metadata documents have the new fields and are updated in real time. - it.skip('does not allow student to access private teacher document', function() { - cy.visit(studentQueryParams); - cy.waitForLoad(); - verifyStudentSeesAsPrivate(); - }); + cy.log('does not allow student to access private teacher document'); + cy.visit(studentQueryParams); + cy.waitForLoad(); + verifyStudentSeesAsPrivate(); - it('allows teacher to share a document', function() { - cy.visit(teacherQueryParams); - cy.waitForLoad(); - clueCanvas.shareCanvas(); - verifySwitch('public'); - }); + cy.log('allows teacher to share a document'); + cy.visit(teacherQueryParams); + cy.waitForLoad(); + clueCanvas.shareCanvas(); + verifySwitch('public'); - // TODO: Reinstate the tests below when all metadata documents have the new fields and are updated in real time. - it.skip('allows student to access public teacher document', function() { - cy.visit(studentQueryParams); - cy.waitForLoad(); - verifyStudentSeesAsPublic(); - }); + cy.log('allows student to access public teacher document'); + cy.visit(studentQueryParams); + cy.waitForLoad(); + verifyStudentSeesAsPublic(); - it('allows teacher to unshare a document', function() { - cy.visit(teacherQueryParams); - cy.waitForLoad(); - clueCanvas.unshareCanvas(); - verifySwitch('private'); - }); + cy.log('allows teacher to unshare a document'); + cy.visit(teacherQueryParams); + cy.waitForLoad(); + clueCanvas.unshareCanvas(); + verifySwitch('private'); }); }); diff --git a/cypress/e2e/functional/teacher_tests/teacher_six_pack_spec.js b/cypress/e2e/functional/teacher_tests/teacher_six_pack_spec.js index d12482c572..3c4db63d23 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_six_pack_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_six_pack_spec.js @@ -4,7 +4,6 @@ let dashboard = new TeacherDashboard(); function beforeTest() { const queryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); dashboard.switchView("Dashboard"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js index 2b4c20ba5c..1637bf6e01 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_sort_work_view_spec.js @@ -18,7 +18,6 @@ const queryParams1 = `${Cypress.config("clueTestqaConfigSubtabsUnitTeacher6")}`; const queryParams2 = `${Cypress.config("qaConfigSubtabsUnitTeacher1")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); @@ -176,10 +175,7 @@ describe('SortWorkView Tests', () => { }); - // TODO: Reinstate the tests below when all metadata documents have the new fields and are being updated in real time. - it.skip("should open Sort Work tab and test sorting by group", () => { - // Clear data before the test so it can be retried and will start with a clean slate - cy.clearQAData('all'); + it("should open Sort Work tab and test sorting by group", () => { const students = ["student:1", "student:2", "student:3", "student:4"]; const studentProblemDocs = [ @@ -303,15 +299,20 @@ describe('SortWorkView Tests', () => { sortWork.getSortWorkItem().contains(exemplarDocs[0]).click(); chatPanel.getChatPanelToggle().click(); chatPanel.addCommentTagAndVerify("Diverging Designs"); + // FIXME: at the moment it is necessary to comment the document twice. + // Search for "exemplar" in document-comment-hooks.ts for an explanation. + cy.wait(100); + chatPanel.addCommentTagAndVerify("Diverging Designs"); cy.log("check that exemplar document is displayed in new tag"); chatPanel.getChatCloseButton().click(); cy.openTopTab('sort-work'); // at the moment this is required to refresh the sort - sortWork.getPrimarySortByMenu().click(); - sortWork.getPrimarySortByNameOption().click(); - sortWork.getPrimarySortByMenu().click(); - sortWork.getPrimarySortByTagOption().click(); + sortWork.getShowForMenu().click(); + sortWork.getShowForInvestigationOption().click(); + sortWork.getShowForMenu().click(); + sortWork.getShowForProblemOption().click(); + cy.get('.section-header-arrow').click({multiple: true}); // Open the sections sortWork.checkDocumentInGroup("Diverging Designs", exemplarDocs[0]); @@ -329,7 +330,12 @@ describe('SortWorkView Tests', () => { sortWork.getPrimarySortByTagOption().click(); cy.get('.section-header-arrow').click({multiple: true}); // Open the sections sortWork.checkDocumentInGroup("Unit Rate", exemplarDocs[0]); - sortWork.checkGroupIsEmpty("Diverging Designs"); + + // FIXME: We haven't implemented support for deleting comments + // what should be true: + // sortWork.checkGroupIsEmpty("Diverging Designs"); + // what currently happens + sortWork.checkDocumentInGroup("Diverging Designs", exemplarDocs[0]); cy.log("run CLUE as a student:1 and join group 6"); runClueAsStudent(students[0], 6); diff --git a/cypress/e2e/functional/teacher_tests/teacher_starreddocument_scroller_spec.js b/cypress/e2e/functional/teacher_tests/teacher_starreddocument_scroller_spec.js index 6027e2e1d6..26dc283863 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_starreddocument_scroller_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_starreddocument_scroller_spec.js @@ -6,7 +6,6 @@ let starred = new StarredTab; function beforeTest() { const queryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_support_spec.js b/cypress/e2e/functional/teacher_tests/teacher_support_spec.js index eb628afc4a..b1957ab228 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_support_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_support_spec.js @@ -8,13 +8,12 @@ const teacherQueryParams = `${Cypress.config("qaUnitTeacher6")}`; const studentQueryParams = `${Cypress.config("qaUnitStudent5")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); cy.wait(4000); } - + function loadStudentSession(params) { cy.visit(params); cy.waitForLoad(); @@ -36,7 +35,7 @@ context('Teacher Support', function() { cy.openTopTab("class-work"); cy.openSection('class-work','workspaces'); resourcesPanel.getCanvasItemTitle('class-work','workspaces').should('contain',title); - + cy.log('verify teacher support is visible in student nav'); loadStudentSession(studentQueryParams); cy.openTopTab("class-work"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_tagged_comments_spec.js b/cypress/e2e/functional/teacher_tests/teacher_tagged_comments_spec.js index e0c57186b1..389c8a40d4 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_tagged_comments_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_tagged_comments_spec.js @@ -4,7 +4,6 @@ let chatPanel = new ChatPanel; function beforeTest() { const queryParams = `${Cypress.config("qaUnitTeacher6Network")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); cy.openTopTab("problems"); diff --git a/cypress/e2e/functional/teacher_tests/teacher_workspace_spec.js b/cypress/e2e/functional/teacher_tests/teacher_workspace_spec.js index 3ba2c8cefd..34ce263c6a 100644 --- a/cypress/e2e/functional/teacher_tests/teacher_workspace_spec.js +++ b/cypress/e2e/functional/teacher_tests/teacher_workspace_spec.js @@ -18,7 +18,6 @@ const teacherQueryParams = `${Cypress.config("qaUnitTeacher6")}`; const studentWorkspaceQueryParams = `${Cypress.config("clueTestqaUnitTeacher6")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); dashboard.switchView("Workspace & Resources"); diff --git a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js index 9a06593069..52c59cfa50 100644 --- a/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js +++ b/cypress/e2e/functional/tile_tests/arrow_annotation_spec.js @@ -39,7 +39,6 @@ const queryParamsQa = `${Cypress.config("qaUnitStudent7Investigation3")}`; // so cypress+chrome simply cannot scroll the container. function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); cy.showOnlyDocumentWorkspace(); diff --git a/cypress/e2e/functional/tile_tests/data_card_tool_spec.js b/cypress/e2e/functional/tile_tests/data_card_tool_spec.js index 2801af6631..c7f18cadf1 100644 --- a/cypress/e2e/functional/tile_tests/data_card_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/data_card_tool_spec.js @@ -8,7 +8,6 @@ let xyplot = new XYPlotToolTile; function beforeTest() { const queryParams = `${Cypress.config("qaMothPlotUnitStudent5")}&mouseSensor`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/datacard_merge_spec.js b/cypress/e2e/functional/tile_tests/datacard_merge_spec.js index 1ed259093a..e98d090247 100644 --- a/cypress/e2e/functional/tile_tests/datacard_merge_spec.js +++ b/cypress/e2e/functional/tile_tests/datacard_merge_spec.js @@ -6,11 +6,17 @@ let dc = new DataCardToolTile; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } +function startFresh() { + cy.clearAllSessionStorage(); + cy.clearAllCookies(); + cy.clearAllLocalStorage(); + beforeTest(); +} + context('Merge Data Card Tool Tile', function () { it("Merge Data Card Tool Tile", () => { beforeTest(); @@ -43,7 +49,7 @@ context('Merge Data Card Tool Tile', function () { dc.getAttrValue(1).eq(1).invoke("val").should("contain", "cat"); cy.log("merges two empty Data Card tool tiles"); - beforeTest(); + startFresh(); clueCanvas.addTile("datacard"); dc.getTile(0).should("exist"); dc.getTileTitle(0).should("have.text", "Card Deck Data 1"); @@ -70,7 +76,7 @@ context('Merge Data Card Tool Tile', function () { dc.getCardNofTotalListing(1).should("have.text", "Card 1 of 2"); cy.log("merges filled-in into empty Data Card tool tile"); - beforeTest(); + startFresh(); clueCanvas.addTile("datacard"); dc.getTile(0).should("exist"); dc.getTileTitle(0).should("have.text", "Card Deck Data 1"); @@ -109,7 +115,7 @@ context('Merge Data Card Tool Tile', function () { dc.getCardNofTotalListing(1).should("have.text", "Card 1 of 2"); cy.log("merges empty into filled-in Data Card tool tile"); - beforeTest(); + startFresh(); clueCanvas.addTile("datacard"); dc.getTile(0).should("exist"); dc.getTileTitle(0).should("have.text", "Card Deck Data 1"); @@ -148,7 +154,7 @@ context('Merge Data Card Tool Tile', function () { dc.getCardNofTotalListing(1).should("have.text", "Card 1 of 2"); cy.log("merges two filled-in Data Card tool tiles"); - beforeTest(); + startFresh(); clueCanvas.addTile("datacard"); dc.getTile(0).should("exist"); dc.getTileTitle(0).should("have.text", "Card Deck Data 1"); @@ -191,7 +197,7 @@ context('Merge Data Card Tool Tile', function () { dc.getCardNofTotalListing(1).should("have.text", "Card 1 of 2"); cy.log("merges datacards with same attribute labels"); - beforeTest(); + startFresh(); clueCanvas.addTile("datacard"); dc.getTile(0).should("exist"); dc.getTileTitle(0).should("have.text", "Card Deck Data 1"); diff --git a/cypress/e2e/functional/tile_tests/diagram_tool_spec.js b/cypress/e2e/functional/tile_tests/diagram_tool_spec.js index 23781901a4..bf18c11bb6 100644 --- a/cypress/e2e/functional/tile_tests/diagram_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/diagram_tool_spec.js @@ -14,7 +14,6 @@ const redoKeystroke = `{${cmdKey}}{shift}z`; function beforeTest() { const queryParams = `${Cypress.config("qaVariablesUnitStudent5")}&mouseSensor`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); cy.showOnlyDocumentWorkspace(); diff --git a/cypress/e2e/functional/tile_tests/drawing_tool_spec.js b/cypress/e2e/functional/tile_tests/drawing_tool_spec.js index 9529daddeb..447b96ca2b 100644 --- a/cypress/e2e/functional/tile_tests/drawing_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/drawing_tool_spec.js @@ -9,8 +9,6 @@ const imageToolTile = new ImageToolTile; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); - cy.visit(queryParams); cy.waitForLoad(); cy.showOnlyDocumentWorkspace(); diff --git a/cypress/e2e/functional/tile_tests/duplicate_tile_spec.js b/cypress/e2e/functional/tile_tests/duplicate_tile_spec.js index 931e17905c..58f5940429 100644 --- a/cypress/e2e/functional/tile_tests/duplicate_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/duplicate_tile_spec.js @@ -14,8 +14,6 @@ let clueCanvas = new ClueCanvas, function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); - cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/expression_tool_spec.js b/cypress/e2e/functional/tile_tests/expression_tool_spec.js index fe276a00da..c7c5104283 100644 --- a/cypress/e2e/functional/tile_tests/expression_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/expression_tool_spec.js @@ -6,7 +6,6 @@ let exp = new ExpressionToolTile; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); //TODO - implement within a curriculum unit diff --git a/cypress/e2e/functional/tile_tests/geometry_table_integraton_test_spec.js b/cypress/e2e/functional/tile_tests/geometry_table_integraton_test_spec.js index 2126a185d3..95cefec597 100644 --- a/cypress/e2e/functional/tile_tests/geometry_table_integraton_test_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_table_integraton_test_spec.js @@ -15,8 +15,6 @@ const y = ['2.5', '5', '1', '0']; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); - cy.visit(queryParams); cy.waitForLoad(); clueCanvas.getInvestigationCanvasTitle().text().as('investigationTitle'); diff --git a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js index 5b31c70aa4..27fe8d1e8c 100644 --- a/cypress/e2e/functional/tile_tests/geometry_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/geometry_tool_spec.js @@ -18,7 +18,6 @@ const ptsDoc = 'Points'; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); cy.collapseResourceTabs(); @@ -277,6 +276,171 @@ context('Geometry Tool', function () { geometryToolTile.getGraphPoint().should("have.length", 0); geometryToolTile.getSelectedGraphPoint().should("have.length", 0); + // Create first polygon from existing points + cy.log('Create first polygon from existing points'); + clueCanvas.clickToolbarButton('geometry', 'point'); + geometryToolTile.clickGraphPosition(0, 0); + geometryToolTile.clickGraphPosition(10, 0); + geometryToolTile.clickGraphPosition(5, 5); + clueCanvas.clickToolbarButton('geometry', 'polygon'); + geometryToolTile.getGraphPoint().should("have.length", 3); + geometryToolTile.getGraphPoint().eq(0).click(); + geometryToolTile.getGraphPoint().eq(1).click(); + geometryToolTile.getGraphPoint().eq(2).click(); + geometryToolTile.getGraphPoint().eq(0).click(); + geometryToolTile.getGraphPolygon().should("have.length", 1); + geometryToolTile.getGraphPoint().should("have.length", 3); + + // Add a point to the existing polygon + cy.log('Add a point to the existing polygon'); + geometryToolTile.clickGraphPosition(10, 0); // Reuse existing point + geometryToolTile.clickGraphPosition(15, 5); + geometryToolTile.clickGraphPosition(5, 5); // Reuse existing point + clueCanvas.clickToolbarButton('geometry', 'polygon'); + // check number of points + geometryToolTile.getGraphPoint().should("have.length", 4); + + // Verify the polygon count is still 1 + geometryToolTile.getGraphPolygon().should("have.length", 1); + + // Create a second polygon that shares the same points as the first + cy.log('Create a second polygon that shares a point with the first'); + clueCanvas.clickToolbarButton('geometry', 'polygon'); + geometryToolTile.clickGraphPosition(15, 10); // new point + geometryToolTile.clickGraphPosition(15, 5); // shared point + geometryToolTile.clickGraphPosition(20, 5); // new point + geometryToolTile.clickGraphPosition(15, 10); // close the polygon + + // Point should be shared + geometryToolTile.getGraphPoint().should("have.length", 6); // New point added + + // Store the original point coordinates for comparison + let originalCx, originalCy; + clueCanvas.clickToolbarButton('geometry', 'select'); + geometryToolTile.clickGraphPosition(15, 5); // shared point + geometryToolTile.getSelectedGraphPoint().then(($point) => { + originalCx = parseFloat($point.attr('cx')); + originalCy = parseFloat($point.attr('cy')); + cy.wrap(originalCx).as('originalCx'); + cy.wrap(originalCy).as('originalCy'); + }); + + // Add length labels to two line segments + // Select line segments by clicking between two points + geometryToolTile.clickGraphPosition(7.5, 5); // Middle of the first segment between (5, 5) and (10, 5) + clueCanvas.clickToolbarButton('geometry', 'label'); + geometryToolTile.getModalTitle().should('contain.text', 'Length'); + geometryToolTile.chooseLabelOption('length'); + + geometryToolTile.clickGraphPosition(15, 7.5); // Middle of the second segment between (15, 5) and (15, 10) + clueCanvas.clickToolbarButton('geometry', 'label'); + geometryToolTile.getModalTitle().should('contain.text', 'Length'); + geometryToolTile.chooseLabelOption('length'); + geometryToolTile.clickGraphPosition(20, 20); // deselect + + let origPointLabel1 = 10.0; + let origPointLabel2 = 5.0; + + // Verify that the first point label is close to 10.0 + geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(origPointLabel1, 0.1); // Allow tolerance for value close to 10.0 + }); + + // Verify that the second point label is close to 5.0 + geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const value = parseFloat(text); + expect(value).to.be.closeTo(origPointLabel2, 0.1); // Allow tolerance for value close to 5.0 + }); + + // Move the point + clueCanvas.clickToolbarButton('geometry', 'select'); + geometryToolTile.clickGraphPosition(15, 5); // shared point + + // Simulate 4 right arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 39 }); + } + + // Simulate 4 up arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 38 }); + } + + // Updated expected values after the point move + let updatedValue1 = origPointLabel1 + 0.4; // 10.4 + let updatedValue2 = origPointLabel2 - 0.4; // 4.6 + + // Verify that the point values changed + geometryToolTile.getSelectedGraphPoint().then(($point) => { + const newPx = parseFloat($point.attr('cx')); + const newPy = parseFloat($point.attr('cy')); + + expect(newPx).to.be.greaterThan(originalCx); + expect(newPy).to.be.lessThan(originalCy); + }); + + // Verify that the first line segment has changed to a value close to 10.4 + geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const lineSegment1 = parseFloat(text); + expect(lineSegment1).to.be.closeTo(updatedValue1, 0.1); // Tolerance for value close to 10.4 + }); + + // Verify that the second line segment has changed to a value close to 4.6 + geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const lineSegment2 = parseFloat(text); + expect(lineSegment2).to.be.closeTo(updatedValue2, 0.1); // Tolerance for value close to 4.6 + }); + + // Move the point back to the original position + // Simulate 4 left arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 37 }); + } + + // Simulate 4 down arrow key presses + for (let i = 0; i < 4; i++) { + geometryToolTile.getSelectedGraphPoint().trigger('keydown', { keyCode: 40 }); + } + + // Verify that the point has returned to its original coordinates + geometryToolTile.getSelectedGraphPoint().then(($point) => { + const resetPx = parseFloat($point.attr('cx')); + const resetPy = parseFloat($point.attr('cy')); + + expect(resetPx).to.equal(originalCx); + expect(resetPy).to.equal(originalCy); + }); + + // Verify that the first line segment has returned to a value close to 10.0 + geometryToolTile.getGraphPointLabel().eq(2).invoke('text').then((text) => { + const lineSegment1 = parseFloat(text); + expect(lineSegment1).to.be.closeTo(origPointLabel1, 0.1); // Tolerance for value close to 10.0 + }); + + // Verify that the second line segment has returned to a value close to 5.0 + geometryToolTile.getGraphPointLabel().eq(3).invoke('text').then((text) => { + const lineSegment2 = parseFloat(text); + expect(lineSegment2).to.be.closeTo(origPointLabel2, 0.1); // Tolerance for value close to 5.0 + }); + + // Verify the point is still shared + geometryToolTile.getGraphPoint().should("have.length", 6); // New point added + + // Delete the first polygon + clueCanvas.clickToolbarButton('geometry', 'select'); + geometryToolTile.clickGraphPosition(5, 3); //click inside the polygon + clueCanvas.clickToolbarButton('geometry', 'delete'); + geometryToolTile.getGraphPolygon().should("have.length", 1); + geometryToolTile.getGraphPoint().should("have.length", 3); + + // Delete the second + clueCanvas.clickToolbarButton('geometry', 'select'); + geometryToolTile.clickGraphPosition(17, 7); // click inside the polygon + clueCanvas.clickToolbarButton('geometry', 'delete'); + geometryToolTile.getGraphPolygon().should("have.length", 0); + geometryToolTile.getGraphPoint().should("have.length", 0); + // Create polygon from existing points clueCanvas.clickToolbarButton('geometry', 'point'); geometryToolTile.clickGraphPosition(0, 0); diff --git a/cypress/e2e/functional/tile_tests/image_tool_spec.js b/cypress/e2e/functional/tile_tests/image_tool_spec.js index 94bb56ecf5..6c68907555 100644 --- a/cypress/e2e/functional/tile_tests/image_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/image_tool_spec.js @@ -10,7 +10,6 @@ let userCanvas = 'Uploaded Images'; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); cy.showOnlyDocumentWorkspace(); diff --git a/cypress/e2e/functional/tile_tests/numberline_tool_spec.js b/cypress/e2e/functional/tile_tests/numberline_tool_spec.js index 335ff2ba01..784798a33f 100644 --- a/cypress/e2e/functional/tile_tests/numberline_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/numberline_tool_spec.js @@ -6,7 +6,6 @@ let numberlineToolTile = new NumberlineToolTile; function beforeTest() { const queryParams = `${Cypress.config("qaNoNavPanelUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/shared_dataset_spec.js b/cypress/e2e/functional/tile_tests/shared_dataset_spec.js index 83fd6a7d05..3988d93152 100644 --- a/cypress/e2e/functional/tile_tests/shared_dataset_spec.js +++ b/cypress/e2e/functional/tile_tests/shared_dataset_spec.js @@ -11,7 +11,6 @@ let xyTile = new XYPlotTile; const queryParams = `${Cypress.config("qaNoNavPanelUnitStudent5")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/shared_variables_test_spec.js b/cypress/e2e/functional/tile_tests/shared_variables_test_spec.js index 934a7b4c00..676c12ae7b 100644 --- a/cypress/e2e/functional/tile_tests/shared_variables_test_spec.js +++ b/cypress/e2e/functional/tile_tests/shared_variables_test_spec.js @@ -10,7 +10,6 @@ const diagramToolTile = new DiagramToolTile; function beforeTest() { const queryParam = `${Cypress.config("qaVariablesUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParam); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/simulator_tile_spec.js b/cypress/e2e/functional/tile_tests/simulator_tile_spec.js index 8c6f4c0321..d9ff5e7e9c 100644 --- a/cypress/e2e/functional/tile_tests/simulator_tile_spec.js +++ b/cypress/e2e/functional/tile_tests/simulator_tile_spec.js @@ -11,7 +11,6 @@ const queryParams2 = `${Cypress.config("qaConfigSubtabsUnitStudent5")}`; const queryParams3 = `${Cypress.config("qaUnitStudent7Investigation3")}`; function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/table_tool_spec.js b/cypress/e2e/functional/tile_tests/table_tool_spec.js index 7f6f1007a4..3583006d83 100644 --- a/cypress/e2e/functional/tile_tests/table_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/table_tool_spec.js @@ -19,7 +19,6 @@ let copyTitle = 'Table Tile Workspace Copy'; function beforeTest() { const queryParams = `${Cypress.config("qaNoNavPanelUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); cy.showOnlyDocumentWorkspace(); diff --git a/cypress/e2e/functional/tile_tests/text_tool_spec.js b/cypress/e2e/functional/tile_tests/text_tool_spec.js index 2111cf8127..8081fa319c 100644 --- a/cypress/e2e/functional/tile_tests/text_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/text_tool_spec.js @@ -10,7 +10,6 @@ let copyTitle = 'Text Tile Workspace Copy'; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js index 828e7af508..3f3dece060 100644 --- a/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js +++ b/cypress/e2e/functional/tile_tests/xy_plot_tool_spec.js @@ -37,7 +37,6 @@ function attributeOfTransform(matcher, n) { function beforeTest(params) { - cy.clearQAData('all'); cy.visit(params); cy.waitForLoad(); } diff --git a/cypress/e2e/smoke/single_student_canvas_test.js b/cypress/e2e/smoke/single_student_canvas_test.js index 99bff1ac36..36949702bd 100644 --- a/cypress/e2e/smoke/single_student_canvas_test.js +++ b/cypress/e2e/smoke/single_student_canvas_test.js @@ -18,7 +18,6 @@ const title = "QA 1.1 Solving a Mystery with Proportional Reasoning"; function beforeTest() { const queryParams = `${Cypress.config("qaUnitStudent5")}`; - cy.clearQAData('all'); cy.visit(queryParams); cy.waitForLoad(); } diff --git a/cypress/run_cypress_test.sh b/cypress/run_cypress_test.sh deleted file mode 100755 index 47fe7341ab..0000000000 --- a/cypress/run_cypress_test.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/bash - -declare -a BRANCH_ARRAY=("master" "production" "dataflow" "dataflow_production") - -npm run start & -wait-on http://localhost:8080 -echo "start TRAVIS_BRANCH=$TRAVIS_BRANCH" -if [[ "$TRAVIS_COMMIT_MESSAGE" == *"[dev-build]"* ]]; then - echo "dev-build" - npm run test:cypress:smoke -elif !(echo $BRANCH_ARRAY | grep -q $TRAVIS_BRANCH); then - echo "elif TRAVIS_BRANCH=$TRAVIS_BRANCH" - npm run test:cypress:branch -else - echo "else TRAVIS_BRANCH=$TRAVIS_BRANCH" - npm run test:cypress:ci -fi - - \ No newline at end of file diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 3a4676718c..bd8806d141 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -52,21 +52,6 @@ Cypress.Commands.add("uploadFile",(selector, filename, type="")=>{ }); }); }); -Cypress.Commands.add("clearQAData", (data)=>{ //clears data from Firebase (currently data='all' is the only one supported) - if (data==='all') { - - cy.visit('?appMode=qa&qaClear=' + data + '&fakeClass=5&fakeUser=student:5'); - // For some reason when using - // cy.get('span', {timeout: 60000}).should('contain','QA Cleared: OK'); - // If there is a test failure then a weird - // error is shown: - // object tested must be an array, a map, an object, a set, a string, - // or a weakset, but undefined given - // The log shows the assertion passing and then shows it failing right after - // using contains fixes this problem. - cy.contains('span', 'QA Cleared: OK', {timeout: 60000}); - } -}); // Login using cy.request, this is faster than using visit, and it makes it possible // to visit a local domain after logging in @@ -117,6 +102,10 @@ Cypress.Commands.add("launchReport", (reportUrl) => { }); Cypress.Commands.add("waitForLoad", () => { cy.get('.version', {timeout: 60000}); + // Log the firebase user id + cy.window().its('stores.db.firebase.userId').then(id => { + cy.log("Firebase uid", id); + }); }); Cypress.Commands.add("deleteWorkspaces",(baseUrl,queryParams)=>{ let primaryWorkspace = new PrimaryWorkspace; diff --git a/cypress/support/e2e.js b/cypress/support/e2e.js index 1ec343511f..8de7870dd5 100644 --- a/cypress/support/e2e.js +++ b/cypress/support/e2e.js @@ -28,10 +28,6 @@ import './commands'; import installLogsCollector from "cypress-terminal-report/src/installLogsCollector"; installLogsCollector(); -// before(function(){ //Can't run this because full tests will not run due to website switching -// cy.clearQAData('all'); -// }); - Cypress.on('uncaught:exception', (err, runnable) => { // returning false here prevents Cypress from // failing the test @@ -54,7 +50,3 @@ if (Cypress.config('hideXHRInCommandLog')) { app.document.head.appendChild(style); } } - -after(function(){ - cy.clearQAData('all'); -}); diff --git a/docs/oauth2.md b/docs/oauth2.md index 4280fb4149..895a2355bf 100644 --- a/docs/oauth2.md +++ b/docs/oauth2.md @@ -86,10 +86,8 @@ CLUE uses this domain for somethings. It seems reasonable for the Portal to alwa - figure out a way to handle branches with the OAuth2 redirects, so we don't have to update the portal configuration each time we want to test a new branch. - simplify params used by a report launch of CLUE. With just the resourceLinkId and a domain it can discover all of the information it needs for the report. This makes the report launch more symmetric with the student launch. Especially if the offering api (or perhaps new resourceLinkId api) provided info about class (or context). The biggest problem with using a single id like that is that either we need a dynamic api where we can specify the shape of the result, or we have to make one request to get the offering info, wait for it, and then make a second request to get the class info. The next bullet can be used to simplify this. - update the portal APIs so it is easier for apps like CLUE to get all of the info they need for a teacher and a student via a single request. But note that if the user is a researcher and not a student or teacher in the class, then the response should not include student names. And it should only include students that have consented for their work to be visible to researchers. -- see if we can simplify the app mode calculation. There are currently a few places to use the token to compute the app mode or whether the app is previewing. It would be easier to separate the `qaClear` from the `qa` app mode. Then we wouldn't need to work with the app mode right at the beginning. This way working with the token could be postponed until `initializeApp` which could figure out the `appMode` itself. But the current clear code is making sure the `appMode` is`qa` so it doesn't accidentally clear out production data. - consider changing urlParams approach so it is an interface to the actual URL. This way if the URL is changed then any code accessing the urlParams will get this updated version. Even better would be to make this observable, so components using url parameters would get re-rendered if the parameter was changed. This way the URL is the source of truth instead of some object that was copied from it. -- move all qaClear code out of AppComponent. The `qaClear=all` is handled outside but other types of qaClear (offering and class) are still handled by the AppComponent. -- something simple like qaClear is still downloading all of the javascript needed by CLUE. It looks like it is also downloading the cms libraries, but it is not. These bundles include the common code that both CLUE and the cms code use. To improve the loading time, we'd need to make CLUE loading even more dynamic. So something like qaClear and the initial OAuth2 load would not have to wait for all of the CLUE javascript to be loaded. If we make this change, we probably want to optimize it so the other core files do start downloading right away, but if all we are doing is qaClear or oauth2 redirecting we just don't wait for them. +- something simple like oauth2 redirecting is still downloading all of the javascript needed by CLUE. It looks like it is also downloading the cms libraries, but it is not. These bundles include the common code that both CLUE and the cms code use. To improve the loading time, we'd need to make CLUE loading even more dynamic. So the initial OAuth2 load would not have to wait for all of the CLUE javascript to be loaded. If we make this change, we probably want to optimize it so the other core files start downloading right away, but if all we are doing is oauth2 redirecting we just don't wait for them. - getClassInfo adds the offeringId to the students and teachers in the ClassInfo object it creates. I think it'd be better to remove this offeringId from these user objects in the ClassInfo. Then anything that needs it will need to access it a different a way. The offeringId is basically a global since CLUE is always launched associated with an assignment (offering). The offeringId of the user ends up being set on the UserModel (which comes from the students in teachers in the ClassInfo). From there it is used by `get activityUrl` to find which of the portalClassOfferings is the active one, to find its activityUrl. It is also used by `getOfferingPath`. This should be refactored because a user does not determine an offering and really it doesn't determine a class either. We should fix things to not make this assumption. In db.ts the offeringId from the UserModel is used to setup the group. This would be the perfect place to pull it from a separate store which which isn't specific to the user. It is also used in app.tsx, in this case the offeringId could be taken from a global store also. So the only unknown spot is all of the places that might be calling getOfferingPath. ## Refactoring out offeringId from UserModel diff --git a/functions-v2/jest.config.js b/functions-v2/jest.config.js index 9c02ec3c77..879629c095 100644 --- a/functions-v2/jest.config.js +++ b/functions-v2/jest.config.js @@ -1,6 +1,20 @@ module.exports = { preset: 'ts-jest', testEnvironment: 'node', + testPathIgnorePatterns: ['lib/', 'node_modules/'], + moduleNameMapper: { + // These are necessary so code imported from ../shared/ will use the same version of + // firebase-admin that the local code does. + // The explicit `^` and `$` are needed so this only matches what we are importing. + // Otherwise it breaks the internal firebase admin code's imports + "^firebase-admin$": "/node_modules/firebase-admin", + "^firebase-admin/firestore$": "/node_modules/firebase-admin/lib/firestore", + "^firebase-admin/app$": "/node_modules/firebase-admin/lib/app", + "^firebase-admin/database$": "/node_modules/firebase-admin/lib/database", + }, + // The tests can't be run in parallel because they are using a shared Firestore and + // Realtime database. + maxWorkers: 1, }; // This is configured here because the clearFirebaseData function from @@ -9,3 +23,4 @@ module.exports = { // The port here should match the port that is set in the emulators // section of firebase.json process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088"; +process.env["FIREBASE_DATABASE_EMULATOR_HOST"]="127.0.0.1:9000"; diff --git a/functions-v2/package-lock.json b/functions-v2/package-lock.json index 3afbb3fb08..b7fc085d58 100644 --- a/functions-v2/package-lock.json +++ b/functions-v2/package-lock.json @@ -1,13 +1,13 @@ { - "name": "functions", + "name": "functions-v2", "lockfileVersion": 3, "requires": true, "packages": { "": { - "name": "functions", + "name": "functions-v2", "dependencies": { "firebase-admin": "^12.1.0", - "firebase-functions": "^5.0.0" + "firebase-functions": "^5.1.1" }, "devDependencies": { "@jest/globals": "^29.7.0", @@ -5823,9 +5823,10 @@ } }, "node_modules/firebase-functions": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/firebase-functions/-/firebase-functions-5.0.1.tgz", - "integrity": "sha512-1m+crtgAR8Tl36gjpM02KCY5zduAejFmDSXvih/DB93apg39f0U/WwRgT7sitGIRqyCcIpktNUbXJv7Y9JOF4A==", + "version": "5.1.1", + "resolved": "https://registry.npmjs.org/firebase-functions/-/firebase-functions-5.1.1.tgz", + "integrity": "sha512-KkyKZE98Leg/C73oRyuUYox04PQeeBThdygMfeX+7t1cmKWYKa/ZieYa89U8GHgED+0mF7m7wfNZOfbURYxIKg==", + "license": "MIT", "dependencies": { "@types/cors": "^2.8.5", "@types/express": "4.17.3", diff --git a/functions-v2/package.json b/functions-v2/package.json index 7f76e6aaab..3ccecef6e9 100644 --- a/functions-v2/package.json +++ b/functions-v2/package.json @@ -5,8 +5,9 @@ "build": "tsc", "build:watch": "tsc --watch", "emulator": "firebase emulators:start --project demo-test", + "emulator:online": "firebase emulators:start", "serve": "npm run build && firebase emulators:start --only functions", - "shell": "npm run build && firebase functions:shell", + "shell": "npm run build && firebase functions:shell --project demo-test", "start": "npm run shell", "test": "jest", "test:emulator": "firebase emulators:start --project demo-test --only firestore,database", @@ -16,10 +17,10 @@ "engines": { "node": "20" }, - "main": "lib/src/index.js", + "main": "lib/functions-v2/src/index.js", "dependencies": { "firebase-admin": "^12.1.0", - "firebase-functions": "^5.0.0" + "firebase-functions": "^5.1.1" }, "devDependencies": { "@jest/globals": "^29.7.0", diff --git a/functions-v2/src/at-midnight.ts b/functions-v2/src/at-midnight.ts new file mode 100644 index 0000000000..ee6b60d367 --- /dev/null +++ b/functions-v2/src/at-midnight.ts @@ -0,0 +1,38 @@ +import {onSchedule} from "firebase-functions/v2/scheduler"; +import * as logger from "firebase-functions/logger"; + +// NOTE: in order for this import from shared to work it is necessary +// to alias "firebase-admin" in tsconfig.json. Otherwise Typescript will +// read the types from the parent node_modules. The parent directory +// has a different version of the firebase dependencies, which cause +// type errors. +import {cleanFirebaseRoots} from "../../shared/clean-firebase-roots"; + +export const atMidnight = onSchedule( + { + // Let the function run for 30 minutes. + // From early testing it looks like the function can delete 500 qa roots + // every 5 minutes. + timeoutSeconds: 1800, + // Run the function at 7am UTC or 12am PDT + schedule: "0 7 * * *", + }, + runAtMidnight +); + +// This function is split out so it can be tested by Jest. The +// firebase-functions-test library doesn't support wrapping onSchedule. +export async function runAtMidnight() { + await cleanFirebaseRoots({ + appMode: "qa", + hoursAgo: 24, + logger, + dryRun: false, + }); + + // When cleanFirebaseRoots is called from a NodeJS script it is + // necessary to call Firebase's deleteApp so no threads are left running. + // Inside of a firebase function according to + // https://stackoverflow.com/a/72933644/3195497 + // it isn't necessary to call deleteApp when the function is done. +} diff --git a/functions-v2/src/index.ts b/functions-v2/src/index.ts index 35c9f89718..94d32083d1 100644 --- a/functions-v2/src/index.ts +++ b/functions-v2/src/index.ts @@ -1,82 +1,5 @@ -import {onDocumentWritten} from "firebase-functions/v2/firestore"; -import * as logger from "firebase-functions/logger"; import * as admin from "firebase-admin"; +export {onUserDocWritten} from "./on-user-doc-written"; +export {atMidnight} from "./at-midnight"; admin.initializeApp(); - -export const updateClassDocNetworksOnUserChange = - onDocumentWritten("{root}/{space}/users/{userId}", async (event) => { - const {root, space, userId} = event.params; - - const classesResult = await admin.firestore() - .collection(`${root}/${space}/classes`) - .where("teachers", "array-contains", userId) - .get(); - - // For every class of this teacher update the networks. - // We could do something more efficient in the case where a network was - // added. Or in the case that networks were not changed. - // That can be figured out by looking at the event.data.before and - // event.data.after documents. - // However to keep the code more simple we just always do the scan - // of classes and teachers. This is required when a network is deleted - // because we need to figure out if another teacher in the class still has - // the deleted network. - - // To optimize this we collect all of the teachers we care about - // and make one request for them instead of requesting the teachers for each - // class separately. - - const teacherIdSet = new Set(); - classesResult.forEach((classDoc) => { - const {teachers} = classDoc.data() as {teachers: string[]}; - if (!Array.isArray(teachers)) return; - teachers.forEach((id) => teacherIdSet.add(id)); - }); - - const teacherIds = [...teacherIdSet]; - - const teacherNetworks: Record = {}; - - // Need to use batching incase the number of teacherIds is larger than 30 - const batchSize = 30; - for (let i = 0; i < teacherIds.length; i += batchSize) { - const batch = teacherIds.slice(i, i + batchSize); - const teachersResult = await admin.firestore() - .collection(`${root}/${space}/users`) - .where("uid", "in", batch) - .get(); - - teachersResult.forEach((teacherDoc) => { - const teacherData = teacherDoc.data(); - teacherNetworks[teacherData.uid] = teacherData.networks; - }); - } - - const classUpdatePromises: Promise[] = []; - classesResult.forEach((classDoc) => { - // Update each class with the networks of each teacher in the class - const {teachers, networks} = classDoc.data() as {teachers: string[], networks: string[] | undefined}; - if (!Array.isArray(teachers)) return; - const classNetworks = new Set(); - teachers.forEach((teacher) => { - const networks = teacherNetworks[teacher]; - if (!networks) return; - networks.forEach((network) => classNetworks.add(network)); - }); - const orderedNetworks = [...classNetworks].sort(); - if (isArrayEqual(networks, orderedNetworks)) return; - - classUpdatePromises.push( - classDoc.ref.update({networks: orderedNetworks}) - ); - }); - - await Promise.all(classUpdatePromises); - - logger.info("User updated", event.document); - }); - -function isArrayEqual(array1: string[] | undefined, array2: string[]) { - return array1?.length === array2.length && array1.every((value, index) => value === array2[index]); -} diff --git a/functions-v2/src/on-user-doc-written.ts b/functions-v2/src/on-user-doc-written.ts new file mode 100644 index 0000000000..f94fe5d527 --- /dev/null +++ b/functions-v2/src/on-user-doc-written.ts @@ -0,0 +1,79 @@ +import {onDocumentWritten} from "firebase-functions/v2/firestore"; +import * as logger from "firebase-functions/logger"; +import * as admin from "firebase-admin"; +import {isArrayEqual} from "./utils"; + +export const onUserDocWritten = + onDocumentWritten("{root}/{space}/users/{userId}", async (event) => { + const {root, space, userId} = event.params; + + const classesResult = await admin.firestore() + .collection(`${root}/${space}/classes`) + .where("teachers", "array-contains", userId) + .get(); + + // For every class of this teacher update the networks. + // We could do something more efficient in the case where a network was + // added. Or in the case that networks were not changed. + // That can be figured out by looking at the event.data.before and + // event.data.after documents. + // However to keep the code more simple we just always do the scan + // of classes and teachers. This is required when a network is deleted + // because we need to figure out if another teacher in the class still has + // the deleted network. + + // To optimize this we collect all of the teachers we care about + // and make one request for them instead of requesting the teachers for each + // class separately. + + const teacherIdSet = new Set(); + classesResult.forEach((classDoc) => { + const {teachers} = classDoc.data() as {teachers: string[]}; + if (!Array.isArray(teachers)) return; + teachers.forEach((id) => teacherIdSet.add(id)); + }); + + const teacherIds = [...teacherIdSet]; + + const teacherNetworks: Record = {}; + + // Need to use batching incase the number of teacherIds is larger than 30 + const batchSize = 30; + for (let i = 0; i < teacherIds.length; i += batchSize) { + const batch = teacherIds.slice(i, i + batchSize); + const teachersResult = await admin.firestore() + .collection(`${root}/${space}/users`) + .where("uid", "in", batch) + .get(); + + teachersResult.forEach((teacherDoc) => { + const teacherData = teacherDoc.data(); + teacherNetworks[teacherData.uid] = teacherData.networks; + }); + } + + const classUpdatePromises: Promise[] = []; + classesResult.forEach((classDoc) => { + // Update each class with the networks of each teacher in the class + const {teachers, networks} = classDoc.data() as {teachers: string[], networks: string[] | undefined}; + if (!Array.isArray(teachers)) return; + const classNetworks = new Set(); + teachers.forEach((teacher) => { + const networks = teacherNetworks[teacher]; + if (!networks) return; + networks.forEach((network) => classNetworks.add(network)); + }); + const orderedNetworks = [...classNetworks].sort(); + if (isArrayEqual(networks, orderedNetworks)) return; + + classUpdatePromises.push( + classDoc.ref.update({networks: orderedNetworks}) + ); + }); + + await Promise.all(classUpdatePromises); + + logger.info("User updated", event.document); + }); + + diff --git a/functions-v2/src/utils.ts b/functions-v2/src/utils.ts new file mode 100644 index 0000000000..118c4acc63 --- /dev/null +++ b/functions-v2/src/utils.ts @@ -0,0 +1,3 @@ +export function isArrayEqual(array1: string[] | undefined, array2: string[]) { + return array1?.length === array2.length && array1.every((value, index) => value === array2[index]); +} diff --git a/functions-v2/test/at-midnight.test.ts b/functions-v2/test/at-midnight.test.ts new file mode 100644 index 0000000000..9639eadafe --- /dev/null +++ b/functions-v2/test/at-midnight.test.ts @@ -0,0 +1,94 @@ +import { + clearFirestoreData, +} from "firebase-functions-test/lib/providers/firestore"; +import {getFirestore, Timestamp} from "firebase-admin/firestore"; +import {getDatabase} from "firebase-admin/database"; +import * as logger from "firebase-functions/logger"; +import {initialize, projectConfig} from "./initialize"; +import {runAtMidnight} from "../src/at-midnight"; + +jest.mock("firebase-functions/logger"); + +const {cleanup} = initialize(); + +const HOUR = 1000 * 60 * 60; + +async function writeFirestoreRoot(lastLaunchMillis = 0) { + const newRoot = getFirestore() + .collection("qa") + .doc(); + + await newRoot.set({ + lastLaunchTime: Timestamp.fromMillis(lastLaunchMillis), + }); + + // Add some sub docs to make sure they are deleted + await newRoot.collection("users").doc().set({ + uid: "test-user", + }); + + return newRoot; +} + +async function writeDatabaseRoot(rootId: string) { + getDatabase().ref("qa").child(rootId).set({someField: "firebase realtime database"}); +} + +// In other tests we use firebase-functions-test to wrap the function. +// In this case it would look like: +// const wrapped = fft.wrap(atMidnight); +// However the wrapper doesn't support onSchedule: +// - The Typescript types don't allow it +// - at run time it doesn't pass the right event: +// https://github.com/firebase/firebase-functions-test/issues/210 +// So instead the code is separated from the onSchedule and called directly. + +describe("atMidnight", () => { + beforeEach(async () => { + await clearFirestoreData(projectConfig); + await getDatabase().ref().set(null); + }); + + test("clean up firestore roots with no database roots", async () => { + await writeFirestoreRoot(); + await runAtMidnight(); + + const roots = await getFirestore().collection("qa").get(); + expect(roots.size).toBe(0); + expect(logger.info) + .toHaveBeenCalledWith("Found 1 roots to delete"); + }); + + test("clean up firestore root and database root", async () => { + const firestoreRoot = await writeFirestoreRoot(); + await writeDatabaseRoot(firestoreRoot.id); + + await runAtMidnight(); + + const fsRoots = await getFirestore().collection("qa").get(); + expect(fsRoots.size).toBe(0); + const dbRoots = await getDatabase().ref("qa").get(); + expect(dbRoots.val()).toEqual(null); + expect(logger.info) + .toHaveBeenCalledWith("Found 1 roots to delete"); + }); + + test("only clean up firestore roots older than 24 hours", async () => { + await writeFirestoreRoot(Date.now() - HOUR); + await writeFirestoreRoot(Date.now() - 12*HOUR); + await writeFirestoreRoot(Date.now() - 23*HOUR); + await writeFirestoreRoot(Date.now() - 25*HOUR); + await writeFirestoreRoot(Date.now() - 48*HOUR); + + await runAtMidnight(); + + const roots = await getFirestore().collection("qa").get(); + expect(roots.size).toBe(3); + expect(logger.info) + .toHaveBeenCalledWith("Found 2 roots to delete"); + }); + + afterAll(async () => { + await cleanup(); + }); +}); diff --git a/functions-v2/test/initialize.ts b/functions-v2/test/initialize.ts new file mode 100644 index 0000000000..61497f4429 --- /dev/null +++ b/functions-v2/test/initialize.ts @@ -0,0 +1,30 @@ +import {deleteApp, initializeApp} from "firebase-admin/app"; +import initializeFFT from "firebase-functions-test"; + +export const projectConfig = { + projectId: "demo-test", + // This URL doesn't have to be valid, it just has to a non empty string + // The actual database host will be picked up from + // FIREBASE_DATABASE_EMULATOR_HOST + // This is defined in jest.config.js + databaseURL: "https://not-a-project.firebaseio.com", +}; + +export function initialize() { + const fft = initializeFFT(projectConfig); + + // When the function is running in the cloud initializeApp is called by index.ts + // In our tests we import the function's module directly so we can call + // initializeApp ourselves. This is beneficial since initializeApp needs to + // be called after initializeFFT above. + const fbApp = initializeApp(); + + const cleanup = async () => { + fft.cleanup(); + // Deleting the Firebase app is necessary for the Jest tests to exit when they + // are complete. FFT creates a testApp which it deletes in cleanup(), but + // we are not using this testApp. + await deleteApp(fbApp); + }; + return {fft, fbApp, cleanup}; +} diff --git a/functions-v2/test/index.test.ts b/functions-v2/test/on-user-doc-written.test.ts similarity index 81% rename from functions-v2/test/index.test.ts rename to functions-v2/test/on-user-doc-written.test.ts index eed08172a9..9bcc865cf5 100644 --- a/functions-v2/test/index.test.ts +++ b/functions-v2/test/on-user-doc-written.test.ts @@ -1,24 +1,14 @@ -import initializeFFT from "firebase-functions-test"; import { clearFirestoreData, } from "firebase-functions-test/lib/providers/firestore"; import * as logger from "firebase-functions/logger"; import * as admin from "firebase-admin"; - -// We cannot import the function here because we need to call -// initializeFFT first in order to set things up before the -// initializeApp is called in the function module. -// import {updateClassDocNetworksOnUserChange} from "../src"; +import {initialize, projectConfig} from "./initialize"; +import {onUserDocWritten} from "../src/on-user-doc-written"; jest.mock("firebase-functions/logger"); -process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088"; -const projectConfig = {projectId: "demo-test"}; -const fft = initializeFFT(projectConfig); - -// We can't initialize the firebase admin here because that -// can only happen once and the function module needs to do it. -// admin.initializeApp(projectConfig); +const {fft, cleanup} = initialize(); type CollectionRef = admin.firestore.CollectionReference< admin.firestore.DocumentData, admin.firestore.DocumentData @@ -29,14 +19,14 @@ describe("functions", () => { await clearFirestoreData(projectConfig); }); - describe("updateClassDocNetworksOnUserChange", () => { + describe("onUserDocWritten", () => { let classesCollection: CollectionRef; let usersCollection: CollectionRef; - function init() { + beforeEach(() => { classesCollection = admin.firestore().collection("demo/test/classes"); usersCollection = admin.firestore().collection("demo/test/users"); - } + }); // eslint-disable-next-line @typescript-eslint/no-explicit-any function writeClassDocs(classDocs: any[]) { @@ -57,17 +47,7 @@ describe("functions", () => { } test("add new network", async () => { - // The function module has to be imported after initializeFFT is called. - // The initializeFFT sets up environment vars so the - // admin.initializeApp() in index.ts will have the same project - // settings. - const {updateClassDocNetworksOnUserChange} = await import("../src"); - - // We can't use beforeEach because this needs to happen after the import. - // And we can't put the import in beforeEach because it would be hard to - // get the imported function typed properly. - init(); - const wrapped = fft.wrap(updateClassDocNetworksOnUserChange); + const wrapped = fft.wrap(onUserDocWritten); const event = { params: { @@ -144,9 +124,7 @@ describe("functions", () => { }); test("remove network", async () => { - const {updateClassDocNetworksOnUserChange} = await import("../src"); - init(); - const wrapped = fft.wrap(updateClassDocNetworksOnUserChange); + const wrapped = fft.wrap(onUserDocWritten); const event = { params: { @@ -225,9 +203,7 @@ describe("functions", () => { // If there is overlap between the networks of the co-teachers then removing // a network from one co-teacher might not change the networks of the class test("no network change in a class", async () => { - const {updateClassDocNetworksOnUserChange} = await import("../src"); - init(); - const wrapped = fft.wrap(updateClassDocNetworksOnUserChange); + const wrapped = fft.wrap(onUserDocWritten); const event = { params: { @@ -304,7 +280,7 @@ describe("functions", () => { }); }); - afterAll(() => { - fft.cleanup(); + afterAll(async () => { + await cleanup(); }); }); diff --git a/functions-v2/tsconfig.json b/functions-v2/tsconfig.json index 4990f4585e..c3db751f1f 100644 --- a/functions-v2/tsconfig.json +++ b/functions-v2/tsconfig.json @@ -11,6 +11,17 @@ // This prevents typescript from trying to include @types from the parent folders. // The types in the parent folders conflict so they break the build. "typeRoots": ["./node_modules/@types"], + "paths": { + // These are necessary so code imported from ../shared/ will use the same version of + // firebase-admin that the local code does. Technically only "firebase-admin/firestore" + // seems to be currently required, but it seems safer to alias all of the admin + // libraries the shared code might be using. + "firebase-admin": ["./node_modules/firebase-admin/lib"], + "firebase-admin/firestore": ["./node_modules/firebase-admin/lib/firestore"], + "firebase-admin/app": ["./node_modules/firebase-admin/lib/app"], + "firebase-admin/database": ["./node_modules/firebase-admin/lib/database"], + }, + }, "compileOnSave": true, "include": [ diff --git a/package.json b/package.json index 72a0dd5a2b..040e8f2011 100644 --- a/package.json +++ b/package.json @@ -97,7 +97,6 @@ "test:coverage:watch": "jest --coverage --watchAll", "test:coverage:cypress:open": "cypress open --env coverage=true", "test:cypress": "cypress run --env testEnv=local", - "test:cypress:ci": "cypress run --env testEnv=local --record", "test:cypress:open": "cypress open --env testEnv=local", "test:cypress:open:disable-gpu": "cross-env ELECTRON_EXTRA_LAUNCH_ARGS=--disable-gpu cypress open --env testEnv=local", "test:cypress:smoke": "cypress run --spec 'cypress/e2e/smoke/single_student_canvas_test.js' --env testEnv=local", diff --git a/scripts/clean-firebase-roots.ts b/scripts/clean-firebase-roots.ts new file mode 100644 index 0000000000..bd39912791 --- /dev/null +++ b/scripts/clean-firebase-roots.ts @@ -0,0 +1,33 @@ +#!/usr/bin/node + +// This script cleans the roots out of the QA or Dev sections of +// the Firebase Realtime database and Firestore. + +// to run this script type the following in the terminal +// cf. https://stackoverflow.com/a/66626333/16328462 +// $ cd scripts +// $ npx tsx clean-firebase-roots.ts + +import admin from "firebase-admin"; +import { deleteApp } from "firebase-admin/app"; +import {cleanFirebaseRoots} from "../shared/clean-firebase-roots.js"; +import { getScriptRootFilePath } from "./lib/script-utils.js"; + +const databaseURL = "https://collaborative-learning-ec215.firebaseio.com"; + +const serviceAccountFile = getScriptRootFilePath("serviceAccountKey.json"); +const credential = admin.credential.cert(serviceAccountFile); +// Initialize the app with a service account, granting admin privileges +const fbApp = admin.initializeApp({ + credential, + databaseURL +}); + +await cleanFirebaseRoots({ + appMode: "qa", + hoursAgo: 90.7, + logger: console, + dryRun: true +}); + +await deleteApp(fbApp); diff --git a/shared/clean-firebase-roots.ts b/shared/clean-firebase-roots.ts new file mode 100644 index 0000000000..7b300e1149 --- /dev/null +++ b/shared/clean-firebase-roots.ts @@ -0,0 +1,50 @@ +// This requires the modern firebase-admin, so it can't be used by functions-v1 +import {Timestamp, getFirestore} from "firebase-admin/firestore"; +import {getDatabase} from "firebase-admin/database"; + +const HOUR = 1000 * 60 * 60; + +interface Logger { + info(...args: any[]): void; +} + +interface Params { + appMode: "qa" | "dev"; + hoursAgo: number; + logger: Logger; + dryRun?: boolean; +} + +export async function cleanFirebaseRoots( + { appMode, hoursAgo, logger, dryRun }: Params +) { + + // Be extra careful so we don't delete production data + if (!["qa", "dev"].includes(appMode)) { + throw new Error(`Invalid appMode ${appMode}`); + } + + // Clean up Firestore and Realtime database roots that haven't been launched in `hoursAgo` hours + const cutOffMillis = Date.now() - hoursAgo*HOUR; + const qaRootsResult = await getFirestore() + .collection(appMode) + .where("lastLaunchTime", "<", Timestamp.fromMillis(cutOffMillis)) + .get(); + + logger.info(`Found ${qaRootsResult.size} roots to delete`); + + // Need to be careful to clean up the root in the realtime database + // first. The record in Firestore is our only way to figure out which + // roots in the realtime database need to be deleted. + for (const root of qaRootsResult.docs) { + // The Realtime database root is deleted first incase it fails. + // This way the root in firestore will remain so we can find it + // and try again later. + const databasePath = `/${appMode}/${root.id}`; + logger.info(`Deleting Realtime Database root: ${databasePath} ...`); + if (!dryRun) await getDatabase().ref(`/${appMode}/${root.id}`).remove(); + logger.info(`Deleting Firestore root: ${root.ref.path} ...`); + if (!dryRun) await getFirestore().recursiveDelete(root.ref); + } + +} diff --git a/src/cms/cms-document-editor.tsx b/src/cms/cms-document-editor.tsx index 666de93e3d..fa9287d36d 100644 --- a/src/cms/cms-document-editor.tsx +++ b/src/cms/cms-document-editor.tsx @@ -22,7 +22,7 @@ interface IState { document?: DocumentModelType; } -const stores = initializeApp("dev", true); +const stores = initializeApp(true); export class CmsDocumentEditor extends React.Component { disposer: IDisposer; diff --git a/src/components/app.tsx b/src/components/app.tsx index 2957e12c2f..8c803fe637 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -17,15 +17,11 @@ import "react-tippy/dist/tippy.css"; import "./app.scss"; interface IProps extends IBaseProps {} -interface IState { - qaCleared: boolean; - qaClearError?: string; -} function resolveAppMode( stores: IStores, - rawFirebaseJWT: string | undefined, - onQAClear?: (result: boolean, err?: string) => void) { + rawFirebaseJWT: string | undefined +) { const { appMode, db, ui} = stores; if (appMode === "authed") { if (rawFirebaseJWT) { @@ -39,18 +35,8 @@ function resolveAppMode( return db.connect({appMode, stores}) .then(() => { if (appMode === "qa") { - const {qaClear, qaGroup} = urlParams; - if (qaClear) { - const cleared = (err?: string) => { - if (onQAClear) { - onQAClear(!err, err); - } - }; - db.clear(qaClear) - .then(() => cleared()) - .catch(cleared); - } - else if (qaGroup) { + const {qaGroup} = urlParams; + if (qaGroup) { db.leaveGroup().then(() => db.joinGroup(qaGroup)); } } @@ -61,7 +47,7 @@ function resolveAppMode( } } -export const authAndConnect = (stores: IStores, onQAClear?: (result: boolean, err?: string) => void) => { +export const authAndConnect = (stores: IStores) => { const {appConfig, curriculumConfig, appMode, db, user, ui} = stores; let rawPortalJWT: string | undefined; @@ -118,7 +104,7 @@ export const authAndConnect = (stores: IStores, onQAClear?: (result: boolean, er stores.loadUnitAndProblem(unitCode, problemId); } } - return resolveAppMode(stores, authenticatedUser.rawFirebaseJWT, onQAClear); + return resolveAppMode(stores, authenticatedUser.rawFirebaseJWT); }) .then(() => { return user.isTeacher @@ -148,19 +134,12 @@ export const authAndConnect = (stores: IStores, onQAClear?: (result: boolean, er @inject("stores") @observer -export class AppComponent extends BaseComponent { - - public state: IState = { - qaCleared: false, - qaClearError: undefined - }; +export class AppComponent extends BaseComponent { constructor(props: IProps) { super(props); - authAndConnect(this.stores, (qaCleared, qaClearError) => { - this.setState({qaCleared, qaClearError}); - }); + authAndConnect(this.stores); } public componentWillUnmount() { @@ -197,15 +176,6 @@ export class AppComponent extends BaseComponent { return this.renderApp(this.renderLoading()); } - if (urlParams.qaClear) { - const {qaCleared, qaClearError} = this.state; - return this.renderApp( - - {qaCleared ? `QA Cleared: ${qaClearError || "OK"}` : "QA Clearing..."} - - ); - } - if (user.isStudent) { if (!user.currentGroupId) { if (appConfig.autoAssignStudentsToIndividualGroups || this.stores.isPreviewing) { diff --git a/src/components/document/sorted-section.tsx b/src/components/document/sorted-section.tsx index 5361b6b936..5b9f158399 100644 --- a/src/components/document/sorted-section.tsx +++ b/src/components/document/sorted-section.tsx @@ -23,7 +23,7 @@ interface IProps { secondarySort: SecondarySortType; } -export const SortedSection: React.FC = observer(function SortedDocuments(props: IProps) { +export const SortedSection: React.FC = observer(function SortedSection(props: IProps) { const { docFilter, documentGroup, idx, secondarySort } = props; const { persistentUI, sortedDocuments } = useStores(); const [showDocuments, setShowDocuments] = useState(false); @@ -51,7 +51,7 @@ export const SortedSection: React.FC = observer(function SortedDocuments const renderUngroupedDocument = (doc: IDocumentMetadata) => { const fullDocument = getDocument(doc.key); - if (!fullDocument) return
; + if (!fullDocument) return
; return { - const [qaCleared, setQACleared] = useState(false); - - useEffect(() => { - clearFirebaseAnonQAUser() - .then(() => setQACleared(true)); - }, []); - - return ( - - {qaCleared ? `QA Cleared: OK` : "QA Clearing..."} - - ); -}; - diff --git a/src/components/thumbnail/simple-document-item.tsx b/src/components/thumbnail/simple-document-item.tsx index ba48cbaa6a..fe0820ea40 100644 --- a/src/components/thumbnail/simple-document-item.tsx +++ b/src/components/thumbnail/simple-document-item.tsx @@ -16,12 +16,11 @@ export const SimpleDocumentItem = ({ document, investigationOrdinal, onSelectDoc const { documents, class: classStore, unit, user } = useStores(); const { uid } = document; const userName = classStore.getUserById(uid)?.displayName; - const investigations = unit.investigations; // TODO: Make it so we don't have to convert investigationOrdinal and problemOrdinal to numbers here? We do so // because the values originate as strings. Changing their types to numbers in the model would make this unnecessary, // but doing that causes errors elsewhere when trying to load documents that aren't associated with a problem. - const investigation = investigations[Number(investigationOrdinal)]; - const problem = investigation?.problems[Number(problemOrdinal) - 1]; + const investigation = unit.getInvestigation(Number(investigationOrdinal)); + const problem = investigation?.getProblem(Number(problemOrdinal)); const title = document.title ? `${userName}: ${document.title}` : `${userName}: ${problem?.title ?? "unknown title"}`; const isPrivate = !isDocumentAccessibleToUser(document, user, documents); diff --git a/src/doc-editor.tsx b/src/doc-editor.tsx index b30a8dbc3d..888e4b9ee2 100644 --- a/src/doc-editor.tsx +++ b/src/doc-editor.tsx @@ -4,13 +4,12 @@ import { ChakraProvider, extendTheme } from "@chakra-ui/react"; import { mode } from "@chakra-ui/theme-tools"; import { DocEditorApp } from "./components/doc-editor/doc-editor-app"; import { DialogComponent } from "./components/utilities/dialog"; -import { urlParams } from "./utilities/url-params"; import { AppProvider, initializeApp } from "./initialize-app"; (window as any).DISABLE_FIREBASE_SYNC = true; -const stores = initializeApp(urlParams.appMode || "dev", true); +const stores = initializeApp(true); // By default Chakra adds some global styles which break some of the // CLUE styles. But removing these styles then breaks the Chakra diff --git a/src/hooks/document-comment-hooks.ts b/src/hooks/document-comment-hooks.ts index 41b4706035..5de83972fe 100644 --- a/src/hooks/document-comment-hooks.ts +++ b/src/hooks/document-comment-hooks.ts @@ -139,6 +139,11 @@ export const usePostDocumentComment = (options?: PostDocumentCommentUseMutationO // FIXME: provide access to remoteContext here so we can update strategies on remote // documents. Alternatively move this into a firebase function instead of doing this // in the client. + // FIXME: in the case of exemplar documents, the metadata document won't exist + // until this mutation happens. That probably means metadataQuery.get() below + // will run before the document has been created so it will return an empty array of + // docs. This is another reason the firebase function approach to keep the strategies + // updated is a better way to go const metadataQuery = firestore.collection("documents") .where("key", "==", documentKey) .where("context_id", "==", context.classHash); diff --git a/src/index.tsx b/src/index.tsx index 26c355f32c..94de827a46 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -3,10 +3,7 @@ import ReactDOM from "react-dom"; import { AppProvider, initializeApp } from "./initialize-app"; import { AppComponent } from "./components/app"; -import { getAppMode } from "./lib/auth"; -import { urlParams } from "./utilities/url-params"; -import { QAClear } from "./components/qa-clear"; -import { getBearerToken, initializeAuthorization } from "./utilities/auth-utils"; +import { initializeAuthorization } from "./utilities/auth-utils"; import { removeLoadingMessage, showLoadingMessage } from "./utilities/loading-utils"; removeLoadingMessage("Loading the application"); @@ -14,24 +11,14 @@ showLoadingMessage("Initializing"); const redirectingToAuthDomain = initializeAuthorization(); if (!redirectingToAuthDomain) { - const host = window.location.host.split(":")[0]; - const appMode = getAppMode(urlParams.appMode, getBearerToken(urlParams), host); + const stores = initializeApp(false); + stores.ui.setShowDemoCreator(!!stores.showDemoCreator); - if (appMode === "qa" && urlParams.qaClear === "all") { - ReactDOM.render( - , - document.getElementById("app") - ); - } else { - const stores = initializeApp(appMode); - stores.ui.setShowDemoCreator(!!stores.showDemoCreator); - - ReactDOM.render( - - - , - document.getElementById("app") - ); - removeLoadingMessage("Initializing"); - } + ReactDOM.render( + + + , + document.getElementById("app") + ); + removeLoadingMessage("Initializing"); } diff --git a/src/initialize-app.tsx b/src/initialize-app.tsx index bdabf244c2..fc43d37fcb 100644 --- a/src/initialize-app.tsx +++ b/src/initialize-app.tsx @@ -15,6 +15,7 @@ import { IStores } from "./models/stores/stores"; import { UserModel } from "./models/stores/user"; import { urlParams } from "./utilities/url-params"; import { getBearerToken } from "./utilities/auth-utils"; +import { getAppMode } from "./lib/auth"; import { DEBUG_STORES } from "./lib/debug"; import { gImageMap } from "./models/image-map"; import PackageJson from "./../package.json"; @@ -37,30 +38,49 @@ const kEnableLivelinessChecking = false; * @param appMode * @returns */ -export const initializeApp = (appMode: AppMode, authoring?: boolean): IStores => { +export const initializeApp = (authoring: boolean): IStores => { const appVersion = PackageJson.version; + const bearerToken = getBearerToken(urlParams); const user = UserModel.create(); - const showDemoCreator = urlParams.demo; - if (showDemoCreator) { - // Override the app mode when the demo creator is being used. - // `authenticate` is still called when the demo creator is shown - // and with an undefined appMode then it will default to `authed` on - // a remote host. This will cause an error as it looks for a token. - // This error was always happening but for some reason before the app - // was still rendering, and now it doesn't. - appMode = "demo"; + + let appMode: AppMode; + let showDemoCreator = false; + if (authoring) { + // Support appMode=qa even when authoring so we can test some features that only show + // up in the qa appMode + appMode = urlParams.appMode === "qa" ? "qa" : "dev"; + } else { + const host = window.location.host.split(":")[0]; + appMode = getAppMode(urlParams.appMode, bearerToken, host); + + showDemoCreator = !!urlParams.demo; + if (showDemoCreator) { + // Override the app mode when the demo creator is being used. + // `authenticate` is still called when the demo creator is shown + // and with an undefined appMode then it will default to `authed` on + // a remote host. This will cause an error as it looks for a token. + // This error was always happening but for some reason before the app + // was still rendering, and now it doesn't. + appMode = "demo"; + } } const demoName = urlParams.demoName; - const isPreviewing = !!(urlParams.domain && urlParams.domain_uid && !getBearerToken(urlParams)); + const isPreviewing = !!(urlParams.domain && urlParams.domain_uid && !bearerToken); const appConfig = AppConfigModel.create(appConfigSnapshot); const stores = createStores( { appMode, appVersion, appConfig, user, showDemoCreator, demoName, isPreviewing }); - if (DEBUG_STORES) { - (window as any).stores = stores; + // Expose the stores if the debug flag is set or we are running in Cypress + const aWindow = window as any; + + // The Cypress docs say you can just check window.Cypress but after a page reload in + // some cases you have to use window.parent.Cypress + const inCypress = aWindow.Cypress || aWindow.parent?.Cypress; + if (DEBUG_STORES || inCypress) { + aWindow.stores = stores; } diff --git a/src/lib/db-clear.ts b/src/lib/db-clear.ts deleted file mode 100644 index 5e0e6f9f84..0000000000 --- a/src/lib/db-clear.ts +++ /dev/null @@ -1,41 +0,0 @@ -import firebase from "firebase/app"; -import { firebaseConfig } from "./firebase-config"; - -// The problem with this approach is that it can't be used to clear a logged in -// user's data. So we should check if QA is ever used to store a logged in user's -// data. -export const clearFirebaseAnonQAUser = async () => { - - // check for already being initialized for tests - if (firebase.apps.length === 0) { - firebase.initializeApp(firebaseConfig()); - } - - let firebaseUser: firebase.User | undefined; - - // We are ignoring the unsubscribe method returned because this function is only - // used in a one-off way. - firebase.auth().onAuthStateChanged((_firebaseUser) => { - if (_firebaseUser) { - firebaseUser = _firebaseUser; - } - }); - - await firebase.auth().signInAnonymously(); - - if (!firebaseUser) { - throw new Error("Firebase User not set after sign in"); - } - - // Notes: - // 1. This path is defined in firebase.ts, there isn't an easy way to use the - // Firebase class without causing additional Firestore connections So it is - // duplicated here. - // 2. Firebase looks up the user from the browser's indexedDb. In cypress this - // is not cleared between test runs. So the same anonymous user will be - // used each time. - const qaUser = firebase.database().ref(`/qa/${firebaseUser.uid}`); - if (qaUser) { - await qaUser.remove(); - } -}; diff --git a/src/lib/db-listeners/index.test.ts b/src/lib/db-listeners/index.test.ts index 44119e3fd1..557c06ada6 100644 --- a/src/lib/db-listeners/index.test.ts +++ b/src/lib/db-listeners/index.test.ts @@ -18,7 +18,9 @@ describe("DBListeners", () => { const db = new DB(); beforeEach(async () => { - await db.connect({appMode: "test", stores, dontStartListeners: true}); + // NOTE: for better or worse this is actually connecting to Firebase for real + // in other tests the Firebase library is mocked. + await db.connect({appMode: "test", stores, dontStartListeners: true, authPersistence: "none"}); }); afterEach(() => { diff --git a/src/lib/db.test.ts b/src/lib/db.test.ts index 554e4a08c7..bfb3e9ccb0 100644 --- a/src/lib/db.test.ts +++ b/src/lib/db.test.ts @@ -22,7 +22,7 @@ const mockFunctions = jest.fn(); const mockAuthStateUnsubscribe = jest.fn(); jest.mock("firebase/app", () => { - return { + const mockFirebase = { apps: [], initializeApp: () => null, auth: () => ({ @@ -30,12 +30,15 @@ jest.mock("firebase/app", () => { callback({ uid: "user-id" }); return mockAuthStateUnsubscribe; }, - signInAnonymously: () => Promise.resolve() + signInAnonymously: () => Promise.resolve(), + setPersistence: (persistence: string) => Promise.resolve() }), database: () => mockDatabase(), firestore: () => mockFirestore(), functions: () => mockFunctions() }; + (mockFirebase.auth as any).Auth = { Persistence: { SESSION: "session"}}; + return mockFirebase; }); type QueryParams = UrlParams.QueryParams; diff --git a/src/lib/db.ts b/src/lib/db.ts index 2cfa1f7b38..826f9f5960 100644 --- a/src/lib/db.ts +++ b/src/lib/db.ts @@ -45,7 +45,10 @@ import { DEBUG_FIRESTORE } from "./debug"; export type IDBConnectOptions = IDBAuthConnectOptions | IDBNonAuthConnectOptions; export interface IDBBaseConnectOptions { stores: IStores; - dontStartListeners?: boolean; // for unit tests + + // for unit tests + dontStartListeners?: boolean; + authPersistence?: firebase.auth.Auth.Persistence; } export interface IDBAuthConnectOptions extends IDBBaseConnectOptions { appMode: "authed"; @@ -64,8 +67,6 @@ export interface GroupUsersMap { [key: string]: string[]; } -export type DBClearLevel = "all" | "class" | "offering"; - export interface ICreateOtherDocumentParams { title?: string; properties?: IDocumentProperties; @@ -166,6 +167,10 @@ export class DB { this.firestore.setFirebaseUser(firebaseUser); if (!options.dontStartListeners) { const { persistentUI, user, db, unitLoadedPromise, exemplarController} = this.stores; + + // Record launch time in Firestore + this.firestore.recordLaunchTime(); + // Start fetching the persistent UI. We want this to happen as early as possible. persistentUI.initializePersistentUISync(user, db); @@ -181,6 +186,10 @@ export class DB { } }); + // SESSION auth persistence is used so each new tab or window gets its own Firebase authentication + // Unless overridden this applies to all app modes (qa, dev, app, auth, test) + firebase.auth().setPersistence(options.authPersistence || firebase.auth.Auth.Persistence.SESSION); + if (options.appMode === "authed") { firebase.auth() .signOut() @@ -780,35 +789,6 @@ export class DB { this.stores.documents.resolveRequiredDocumentPromiseWithNull(document.type); } - public clear(level: DBClearLevel) { - return new Promise((resolve, reject) => { - const {user} = this.stores; - const clearPath = (path?: string) => { - this.firebase.ref(path).remove().then(resolve).catch(reject); - }; - - if (this.stores.appMode !== "qa") { - return reject("db#clear is only available in qa mode"); - } - - if (level === "all") { - return reject("clearing 'all' is handled by clearFirebaseAnonQAUser"); - } - - switch (level) { - case "class": - clearPath(this.firebase.getClassPath(user)); - break; - case "offering": - clearPath(this.firebase.getOfferingPath(user)); - break; - default: - reject(`Invalid clear level: ${level}`); - break; - } - }); - } - public createDocumentModelFromProblemMetadata( type: ProblemOrPlanningDocumentType, userId: string, metadata: DBOfferingUserProblemDocument) { diff --git a/src/lib/firestore.ts b/src/lib/firestore.ts index f62c013b4f..cb867bbd8c 100644 --- a/src/lib/firestore.ts +++ b/src/lib/firestore.ts @@ -160,4 +160,29 @@ export class Firestore { const userDoc = await this.doc(`users/${uid}`).get(); return userDoc.data() as UserDocument | undefined; } + + /** + * Record the lastLaunchTime in the Firestore root. + * + * This is only recorded for dev, qa, test, and demo appModes. + * In the dev, qa, and test modes each user has their own root or a new root + * is created on each test. + * In the demo mode there could be lots of users launching in the same root + * but this number should be manageable, and it will be useful to keep track + * of how old various demo roots are. + * In the auth (portal launch) case lots of users will be launching the same + * root so the lastLaunchTime would be updated too frequently. Also we can use + * logs and portal information to find the last portal launch. + * + * @returns a promise that resolves when the lastLaunchTime has been updated + */ + public async recordLaunchTime() { + const { appMode } = this.db.stores; + + if (!["dev", "qa", "test", "demo"].includes(appMode)) { + return; + } + + return this.doc("").set({lastLaunchTime: this.timestamp()}, {merge: true}); + } } diff --git a/src/models/stores/document-group.ts b/src/models/stores/document-group.ts index 24db168fdd..8bb92658a7 100644 --- a/src/models/stores/document-group.ts +++ b/src/models/stores/document-group.ts @@ -45,7 +45,6 @@ export class DocumentGroup { stores: ISortedDocumentsStores; label: string; documents: IDocumentMetadata[]; - firestoreTagDocumentMap = new Map>(); icon?: FC>; constructor(props: IDocumentGroup) { @@ -98,7 +97,7 @@ export class DocumentGroup { get byStrategy(): DocumentGroup[] { const commentTags = this.stores.appConfig.commentTags; - const tagsWithDocs = getTagsWithDocs(this.documents, commentTags, this.firestoreTagDocumentMap); + const tagsWithDocs = getTagsWithDocs(this.documents, commentTags); const sortedDocsArr: DocumentGroup[] = []; Object.entries(tagsWithDocs).forEach((tagKeyAndValObj) => { diff --git a/src/models/stores/sorted-documents.ts b/src/models/stores/sorted-documents.ts index 1318b1b8d0..6b7cbefd40 100644 --- a/src/models/stores/sorted-documents.ts +++ b/src/models/stores/sorted-documents.ts @@ -43,7 +43,6 @@ export interface ISortedDocumentsStores { export class SortedDocuments { stores: ISortedDocumentsStores; - firestoreTagDocumentMap = new Map>(); firestoreMetadataDocs: IObservableArray = observable.array([]); constructor(stores: ISortedDocumentsStores) { @@ -113,7 +112,7 @@ export class SortedDocuments { get byStrategy(): DocumentGroup[] { const commentTags = this.commentTags; - const tagsWithDocs = getTagsWithDocs(this.firestoreMetadataDocs, commentTags, this.firestoreTagDocumentMap); + const tagsWithDocs = getTagsWithDocs(this.firestoreMetadataDocs, commentTags); const sortedDocsArr: DocumentGroup[] = []; Object.entries(tagsWithDocs).forEach((tagKeyAndValObj) => { @@ -208,20 +207,34 @@ export class SortedDocuments { tools.push("Sparrow"); } - const metadata: IDocumentMetadata = { - uid: doc.uid, - type: doc.type, - key: doc.key, - createdAt: doc.createdAt, - title: doc.title, - properties: undefined, - tools, - strategies: exemplarStrategy ? [exemplarStrategy] : [], - investigation: doc.investigation, - problem: doc.problem, - unit: doc.unit - }; - docsArray.push(metadata); + const authoredStrategies = exemplarStrategy ? [exemplarStrategy] : []; + + const existingMetadataDoc = docsArray.find(metadataDoc => doc.key === metadataDoc.key); + if (existingMetadataDoc) { + // This will happen if a user comments on a exemplar + // That will create a metadata document in Firestore. + // So in this case we want to update this existing metadata document so we don't + // create a duplicate one + const userStrategies = existingMetadataDoc.strategies || []; + existingMetadataDoc.tools = tools; + existingMetadataDoc.strategies = [...new Set([...authoredStrategies, ...userStrategies])]; + } else { + const metadata: IDocumentMetadata = { + uid: doc.uid, + type: doc.type, + key: doc.key, + createdAt: doc.createdAt, + title: doc.title, + properties: undefined, + tools, + strategies: exemplarStrategy ? [exemplarStrategy] : [], + investigation: doc.investigation, + problem: doc.problem, + unit: doc.unit + }; + docsArray.push(metadata); + } + }); runInAction(() => { diff --git a/src/utilities/sort-document-utils.ts b/src/utilities/sort-document-utils.ts index 60175323b0..c742a23ec9 100644 --- a/src/utilities/sort-document-utils.ts +++ b/src/utilities/sort-document-utils.ts @@ -71,8 +71,10 @@ export const sortNameSectionLabels = (docMapKeys: string[]) => { }); }; -export const getTagsWithDocs = (documents: IDocumentMetadata[], commentTags: Record|undefined, - firestoreTagDocumentMap: Map>) => { +export const getTagsWithDocs = ( + documents: IDocumentMetadata[], + commentTags: Record|undefined, +) => { const tagsWithDocs: Record = {}; if (commentTags) { for (const key of Object.keys(commentTags)) { @@ -93,18 +95,7 @@ export const getTagsWithDocs = (documents: IDocumentMetadata[], commentTags: Rec // in store to find "Documents with no comments" then place those doc keys to "Not Tagged" const uniqueDocKeysWithTags = new Set(); - // grouping documents based on firestore comment tags - firestoreTagDocumentMap.forEach((docKeysSet, tag) => { - const docKeysArray = Array.from(docKeysSet); // Convert the Set to an array - if (tagsWithDocs[tag]) { - docKeysSet.forEach((docKey: string) =>{ - uniqueDocKeysWithTags.add(docKey); - }); - tagsWithDocs[tag].docKeysFoundWithTag = docKeysArray; - } - }); - - // adding in (exemplar) documents with authored tags + // Sort documents into their groups documents.forEach(doc => { doc.strategies?.forEach(strategy => { if (tagsWithDocs[strategy]) { diff --git a/src/utilities/url-params.ts b/src/utilities/url-params.ts index c9d6e0e4b0..eee2ec93e3 100644 --- a/src/utilities/url-params.ts +++ b/src/utilities/url-params.ts @@ -1,6 +1,5 @@ import { ParsedQuery, parse } from "query-string"; import { AppMode, AppModes } from "../models/stores/store-types"; -import { DBClearLevel } from "../lib/db"; export interface QueryParams { // appMode is "authed", "test" or "dev" with the default of dev @@ -79,8 +78,6 @@ export interface QueryParams { // group id for qa qaGroup?: string; - // db level to clear for qa - qaClear?: DBClearLevel; // direct firebase realtime database access to the emulator firebase?: string; // "emulator" or host:port url