Fix part of #17712: Add curriculum admin topic management acceptance tests #19914

Vir-8 · 2024-03-10T15:41:59Z

Overview

This PR fixes part of Acceptance Testing - covering all the Creator's and Contributor's CUJs. #17712.
This PR does the following: Adds acceptances tests for topic management, for the curriculum admin role. Tests the ability of the curriculum admin to create topics, subtopics, skills, stories, and chapters, as well as publish these creations.

Essential Checklist

The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
I have followed the instructions for making a code change.
I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I don't have permissions to assign reviewers directly).
The linter/Karma presubmit checks pass on my local machine, and my PR follows the coding style guide).
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)

Proof that changes are correct

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

… stories

…ublish stories

…-topic-management-acceptance-test

oppiabot · 2024-03-10T15:42:01Z

Hi @Vir-8, can you complete the following:

The body of this PR is missing the required description, please update the body with a description of what this PR does.
The karma and linter checklist has not been checked, please make sure to run the frontend tests and lint tests before pushing.
Thanks!

oppiabot · 2024-03-10T15:42:11Z

Hi @Vir-8 please assign the required reviewer(s) for this PR. Thanks!

oppiabot · 2024-03-12T16:39:39Z

Hi @Vir-8. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

…-topic-management-acceptance-test

Vir-8 · 2024-04-18T17:33:59Z

@seanlip I've addressed all your comments and really hope it's better, I'm aware that it's failing on CI but I'm unable to reproduce the error manually and have made a debugging doc at #20184 regarding the same.

I don't see any problems in the code, PTAL meanwhile as the issue might cause extended delays.

seanlip

I have barely any concerns with the code, it looks good in general. Thanks! Please fix the minor nits and feel free to get this submitted.

Sorry to hear about the failing test though. In general it would be great to have screenshot functionality so we can take a screenshot at the failure point in GitHub Actions....or a video perhaps. Maybe one thing you can do is to try doing other actions on the page to get a sense of what's there and what's not -- basically try to log as much info as you can about the page to tell you something about its structure at the point the test fails.

seanlip · 2024-04-19T18:36:42Z

...r-acceptance-tests/spec/curriculum-admin-tests/create-and-publish-topics-and-stories.spec.ts

+        'Test Subtopic 1',
+        'Test Topic 1'
+      );
+      await curriculumAdmin.addSkillToDiagnostingTestsOfTopic(


addSkillToDiagnosticTest(....)

(note: there is 1 diagnostic test for the math classroom, and it covers all topics)

seanlip · 2024-04-19T18:37:01Z

...r-acceptance-tests/spec/curriculum-admin-tests/create-and-publish-topics-and-stories.spec.ts

+        explorationId,
+        'Test Topic 1'
+      );
+      await curriculumAdmin.expectTopicToBePublishedInTopicAndSkillsDashboard(


It's TopicsAndSkillsDashboard

seanlip · 2024-04-19T18:38:21Z

core/tests/puppeteer-acceptance-tests/user-utilities/curriculum-admin-utils.ts

+
+export class CurriculumAdmin extends BaseUser {
+  /**
+   * Function for navigating to the topic and skills dashboard page.


You can omit "Function for ...", "Function to ..." -- try to document these similar to what we do in Python, start the docstrings with a simple verb.

oppiabot · 2024-04-19T18:46:57Z

Hi @Vir-8, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

oppiabot · 2024-04-20T14:39:50Z

Hi @Vir-8. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

…-topic-management-acceptance-test

Vir-8 · 2024-04-26T05:37:24Z

@seanlip The CI error has been fixed with a simple await, feel free to add this PR to the merge queue

jnvtnguyen · 2024-04-26T06:09:48Z

core/tests/puppeteer-acceptance-tests/user-utilities/curriculum-admin-utils.ts

+  /**
+   * Navigate to the creator dashboard page.
+   */
+  async navigateToCreatorDashboardPage(): Promise<void> {


Aren't we duplicating code here? Thought this was apart of exploration editor utils already.

jnvtnguyen · 2024-04-26T06:11:45Z

core/tests/puppeteer-acceptance-tests/user-utilities/curriculum-admin-utils.ts

+  /**
+   * Create an exploration as a curriculum admin.
+   */
+  async createAndPublishExploration(


Same here, I would expect this to be in the exploration editor utils, unless I'm missing some context.

We're testing the curriculum admin role here though - not the exploration editor. Curriculum admins can also create and publish explorations. Ditto above, seems to be recently pushed code.

Sorry to clarify AFAIK we are trying to separate user behaviors. Yes a curriculum admin can create and publish explorations but so can every user, we wouldn't want to duplicate this for every user role. We use composition which allows a user to have multiple classes. ATM the exploration editor class is composed by default so any function you put in there can be ran by any created user.

Unless this function has something specific to the curriculum admin role we should separate it into the exploration editor class.

Ah okay, thanks for explaining, so are you suggesting to create a separate exploration editor user in the spec to deal with exploration creation if I'm not misunderstanding? I didn't quite get what you meant by a user having multiple classes.

No it's fine to have one user just move these functions over to the other file and concat the types.

E.g. when we you declare curriculumAdmin: CurriculumAdmin & ExplorationEditor

Done, thanks!

…cation

jnvtnguyen

Thanks, LGTM.

oppiabot · 2024-04-26T18:06:06Z

Hi @Vir-8, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

…ance tests (oppia#19914) * Create acceptance test files and implement functionality to add skills * Add functionality to create questions * Add functionality to create explorations * Add thumbnail image and topic creation functionality * Add functionality to create subtopics, stories, chapters, and publish stories * Write the acceptance tests to create topics, stories, chapters, and publish stories * Migrate curriculum admin tests to TypeScript * Add comments and fix tests * Merge remote-tracking branch 'upstream/develop' into curriculum-admin-topic-management-acceptance-test * Fix linting issues * Fix utility functions * Fix tests by adding timeouts for improved stability * Clean code * Fix typescript issues and set the main page to the active tab * Change comments for clarity * Add expectation tests * Fix linting issues * Improve extracting exploration ID and handle exceptions * Correct wordings and input texts * Remove redundant wait and make question creation functionality independent of user flow * Add comments explaining timeouts and generalize functionality for timeout event * Fix opening skill editor before question creation * Refactor story creation to combine functions following user flow * Modify functionalities to use objects for user input information * Fix handling necessary question creation * Complete topic creation functionality including publishing topic * Appropriately publish topics, update expectance test and replace timeouts * Add comments for clarity * Move helper function to spec file * Clean code and add comments * Fix linting issues * Minimize flaky behaviour appropriately * Clean code * Modify tests to emulate mobile user behaviour * Adjust utility to publish topics for mobile functionality * Remove test timeouts * Improve saving topic draft flow * Fix new tab flake and finalize mobile utility functions * Move exploration ID extraction function to base user utility service * Remove objects and pass data as explicit arguments * Fix linting issues * Address comments and improve clarity * Improve user utility functionality * Reduce flakiness and improve naming * Fix linter issues * Fix flake and improve naming * Fix typos and improve comments * Fix merge issues and update suite * Attempt to fix CI error * Create explorations from exploration editor utils to avoid code duplication * Remove unused variables

Vir-8 added 11 commits March 8, 2024 18:00

Create acceptance test files and implement functionality to add skills

399afea

Add functionality to create questions

b275aaa

Add functionality to create explorations

8cae21f

Add thumbnail image and topic creation functionality

b1244cd

Add functionality to create subtopics, stories, chapters, and publish…

ade4b0c

… stories

Write the acceptance tests to create topics, stories, chapters, and p…

b7d6f82

…ublish stories

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

bd66334

…-topic-management-acceptance-test

Migrate curriculum admin tests to TypeScript

8ea5c1c

Add comments and fix tests

b958c68

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

b0a989f

…-topic-management-acceptance-test

Fix linting issues

d244c73

oppiabot bot assigned Vir-8 Mar 10, 2024

Fix utility functions

f42e689

AFZL210 mentioned this pull request Mar 11, 2024

Acceptance Testing - covering all the Creator's and Contributor's CUJs. #17712

Open

13 tasks

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Mar 12, 2024

Vir-8 added 4 commits March 14, 2024 10:48

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

b290f0c

…-topic-management-acceptance-test

Fix tests by adding timeouts for improved stability

207b547

Clean code

9999588

Fix typescript issues and set the main page to the active tab

8c3026c

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Mar 14, 2024

Vir-8 added 5 commits March 14, 2024 18:57

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

2c08365

…-topic-management-acceptance-test

Change comments for clarity

afcf42c

Add expectation tests

bc21ab9

Fix linting issues

9850016

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

928a9bd

…-topic-management-acceptance-test

Vir-8 marked this pull request as ready for review March 14, 2024 18:54

Vir-8 requested a review from a team as a code owner March 14, 2024 18:54

Vir-8 added 2 commits April 18, 2024 17:20

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

1642abc

…-topic-management-acceptance-test

Fix linter issues

8ff01e4

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Apr 18, 2024

Fix flake and improve naming

c1f7a12

Vir-8 assigned seanlip Apr 18, 2024

seanlip approved these changes Apr 19, 2024

View reviewed changes

seanlip removed their assignment Apr 19, 2024

oppiabot bot added the PR: LGTM label Apr 19, 2024

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Apr 20, 2024

Vir-8 added 3 commits April 24, 2024 20:53

Fix typos and improve comments

3fcb853

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

e23bd60

…-topic-management-acceptance-test

Fix merge issues and update suite

a6245c8

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Apr 24, 2024

Vir-8 added 2 commits April 25, 2024 22:27

Attempt to fix CI error

de85bc0

Merge remote-tracking branch 'upstream/develop' into curriculum-admin…

5372a33

…-topic-management-acceptance-test

jnvtnguyen reviewed Apr 26, 2024

View reviewed changes

Vir-8 added 2 commits April 26, 2024 19:36

Create explorations from exploration editor utils to avoid code dupli…

5847c97

…cation

Remove unused variables

b12f5b1

Vir-8 requested a review from jnvtnguyen April 26, 2024 17:54

jnvtnguyen approved these changes Apr 26, 2024

View reviewed changes

seanlip added this pull request to the merge queue Apr 26, 2024

Merged via the queue into oppia:develop with commit e06f8d0 Apr 26, 2024
81 checks passed

Akhilesh-max mentioned this pull request May 29, 2024

[GSoC'24 M1+M2]: Acceptance Test Coverage for Learner's and Admin's CUJs. #20374

Open

59 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #17712: Add curriculum admin topic management acceptance tests #19914

Fix part of #17712: Add curriculum admin topic management acceptance tests #19914

Vir-8 commented Mar 10, 2024 •

edited

Loading

oppiabot bot commented Mar 10, 2024

oppiabot bot commented Mar 10, 2024

oppiabot bot commented Mar 12, 2024

Vir-8 commented Apr 18, 2024

seanlip left a comment

seanlip Apr 19, 2024

Vir-8 Apr 24, 2024

seanlip Apr 19, 2024

Vir-8 Apr 24, 2024

seanlip Apr 19, 2024

Vir-8 Apr 24, 2024

oppiabot bot commented Apr 19, 2024

oppiabot bot commented Apr 20, 2024

Vir-8 commented Apr 26, 2024

jnvtnguyen Apr 26, 2024

Vir-8 Apr 26, 2024

jnvtnguyen Apr 26, 2024

Vir-8 Apr 26, 2024

jnvtnguyen Apr 26, 2024

jnvtnguyen Apr 26, 2024

Vir-8 Apr 26, 2024

jnvtnguyen Apr 26, 2024

jnvtnguyen Apr 26, 2024

Vir-8 Apr 26, 2024

jnvtnguyen left a comment

oppiabot bot commented Apr 26, 2024

Fix part of #17712: Add curriculum admin topic management acceptance tests #19914

Fix part of #17712: Add curriculum admin topic management acceptance tests #19914

Conversation

Vir-8 commented Mar 10, 2024 • edited Loading

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented Mar 10, 2024

oppiabot bot commented Mar 10, 2024

oppiabot bot commented Mar 12, 2024

Vir-8 commented Apr 18, 2024

seanlip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oppiabot bot commented Apr 19, 2024

oppiabot bot commented Apr 20, 2024

Vir-8 commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnvtnguyen left a comment

Choose a reason for hiding this comment

oppiabot bot commented Apr 26, 2024

Vir-8 commented Mar 10, 2024 •

edited

Loading