OCLOMRS-791: Automated Test: New organization creation form #716
Conversation
|
@jwnasambu Great start here but I have noticed a few things,
|
|
@hadijahkyampeire thanks for the review I have fixed it! |
There was a problem hiding this comment.
@jwnasambu Thanks for this. It looks like you followed the patterns from the dictionary stuff correctly. I've added some suggested changes to make things a little cleaner. I'd suggest that we also need some tests to confirm:
- When creating an organisation, the user must provide an organisation id
- When creating an organisation, the user must provide an organisation name
These can be modeled on the tests for requiring a username and password from login.feature.
| @@ -0,0 +1,66 @@ | |||
| /// <reference types="cypress" /> | |||
There was a problem hiding this comment.
This file should be in cypress/support/step_definitions/organisation/edit/edit.ts)
There was a problem hiding this comment.
any reasons for repeating edit/edit, wouldn't cypress/support/step_definitions/organisation/edit.js be better?
There was a problem hiding this comment.
It is repetitive; unfortunately, it's also necessary for the way the preprocessor is configured. More exactly, it just loads all the scripts in /create/edit/*.{js,ts}. So the file could be called index.ts, junk.ts, etc. edit/edit.ts just makes things clear (so it has the same name as the feature file.
| cy.visit(`/orgs/${organisationId}/edit/`) | ||
| ); | ||
|
|
||
| When(/the user selects "(.+)" public_access => { |
There was a problem hiding this comment.
This is where the error you're getting occurs. This should be:
| When(/the user selects "(.+)" public_access => { | |
| When(/the user selects "(.+)"/, public_access => { |
| } | ||
| }); | ||
| }); | ||
|
|
| cy.deleteOrganisation(organisationID, user, true); | ||
| } | ||
| }); | ||
| }); |
| import { customAlphabet } from "nanoid"; | ||
| import { isLoggedIn } from "../../../utils"; | ||
|
|
||
| const user = Cypress.env("USERNAME") || "admin"; |
There was a problem hiding this comment.
I don't think we need the user here?
| ); | ||
|
|
||
| Before({ tags: "@organisation" }, () => { | ||
| organisationID = `TD-${nanoid()}`; |
There was a problem hiding this comment.
| organisationID = `TD-${nanoid()}`; | |
| organisationID = `Org-${nanoid()}`; |
TD = "Test Dictionary"
There was a problem hiding this comment.
Thanks so much @ibacher for the clear description.
| ); | ||
|
|
||
| Before({ tags: "@organisation" }, () => { | ||
| organisationId = `TD-${nanoid()}`; |
There was a problem hiding this comment.
| organisationId = `TD-${nanoid()}`; | |
| organisationId = `Org-${nanoid()}`; |
| Given(/a (public) organisation exists/, type => { | ||
| cy.createOrganisation(organisationId, user, type === "public"); | ||
| }); |
There was a problem hiding this comment.
| Given(/a (public) organisation exists/, type => { | |
| cy.createOrganisation(organisationId, user, type === "public"); | |
| }); | |
| Given("a public organisation exists", () => { | |
| cy.createOrganisation(organisationId, user, true); | |
| }); |
|
@ibacher thanks for the review am gonna fix the changes shortly. |
… into OCLOMRS-791
| @organisation | ||
| Scenario: The user should be able to create a new organisation | ||
| Given the user is on the create new organisation page | ||
| When the user enters the organisation information |
There was a problem hiding this comment.
i would prefer Given, When, Then to have the same starting line for clarity, The sub steps can be indented
There was a problem hiding this comment.
@k-joseph thanks for the review. Let me fix right away.
There was a problem hiding this comment.
@kaweesi I actually prefer this style. It keeps the actual steps aligned with each other, which I think is more useful for figuring out what's important. As a bonus, it also keeps And indented under the Given, When, Then which makes it easier, at least to me, to follow whats going on. In short, for me at least, it's easier to visually see what each scenario does.
|
@ibacher, @kaweesi and @hadijahkyampeire I removed the edit file file since its not what the ticket needs. Kindly feel free to review the PR again. |
|
@jwnasambu just a small point:-while resolving these conflicts, try to keep what is coming from |
| Then the new organisation should be created | ||
|
|
||
| @organisation | ||
| Scenario: The user should be able to create a public organisation |
There was a problem hiding this comment.
Shouldn't we have a similar scenario for a private organisation? Apart from that, I think this PR is ready-to-merge.
ibacher
changed the title
JIRA TICKET NAME:
Automated Test: New organization creation form
Summary:
Created the feature files and steps description file create organisation.