Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e proposal edit mock structure #2692

Merged

Conversation

vibros68
Copy link
Contributor

@vibros68 vibros68 commented Dec 16, 2021

Closes #2645

This diff implement mock data for edit proposal page. So the teste2e
will not submit to the server. This new mock structure introduced by
#2624.

Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey mate, thanks for the PR. Noticed that your first test case hits the /comments/v1/comments on the backend. Also, left some inline comments to improve test readability and flow.

Notice that we already use middlewares for making request aliases, so you can use middleware's alias instead of cy.route().

cy.findByTestId(/record-edit-button/i).click();
cy.findByRole("button", { name: /submit/i }).should("be.disabled");
cy.findByTestId("text-area").type(newDescription);
cy.route("POST", "/api/records/v1/edit").as("editProposal");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove it, since we are already using middlewares.

Comment on lines 103 to 112
const { name, description, startDate, endDate, amount, domain } =
buildProposal();
const { files } = makeProposal({
name,
markdown: description,
startdate: startDate,
enddate: endDate,
amount,
domain
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember, records and proposals are different concepts. This should handle records, not proposals. You should pass files as a test parameter, if you need it.

return { record };
}

export function editRecordReply({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit record should not return version 1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment wasn't addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is addressed. If we do not pass the version param. The function will generate a randomly greater than 1 value. The code is here:
const version = recordVersion || faker.datatype.number(6) + 1;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, my bad

@@ -97,7 +97,7 @@ export function Record({
const token = recordToken || Token();
const status = recordStatus || faker.datatype.number(3) + 1;
const state = recordState || faker.datatype.number(1) + 1;
const user = author instanceof User || new User();
const user = author || new User();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? The instanceof comparison is the whole purpose of using classes on data generators 😁 . I guess it's better if we take advantage of this feature.

Copy link
Contributor Author

@vibros68 vibros68 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instanceof work on the normal function. But after pass via cy middleware function, it is not working right and the result is always be false

export function meReply({ testParams: { userType, verifyIdentity } }) {
const user = userByType(userType);
export function meReply({ testParams: { userType, verifyIdentity, user } }) {
if (!user) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where you can use the instanceof operator, where you can assert if user has User props.

Comment on lines 95 to 96
cy.intercept("/api/records/v1/details").as("details");
cy.intercept("/api/pi/v1/summaries").as("summaries");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Comment on lines 3 to 4
import { makeProposal } from "../../utils";
import { buildProposal } from "../generate";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proposals != records

cy.findByTestId(/record-edit-button/i).should("not.exist");
}
);
it("shouldn't be editable a proposal if not be the owner", () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it("shouldn't be editable a proposal if not be the owner", () => {
it("shouldn't allow editing if user is not the proposal owner", () => {


beforeEach(() => {
describe("Proposal Edit", () => {
it("should be editable a public proposal as a proposal owner", () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it("should be editable a public proposal as a proposal owner", () => {
it("should allow proposal owner to edit its own public proposal", () => {

it("shouldn't be editable without making any changes", () => {
cy.server();
const user = userByType(USER_TYPE_ADMIN);
cy.log(user, user instanceof User);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log

Copy link
Member

@victorgcramos victorgcramos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just need to fix https://github.com/decred/politeiagui/pull/2692/files#r770595023 and we should be good to go! Good job!

Screen Shot 2021-12-20 at 12 46 05 PM

@vibros68
Copy link
Contributor Author

Thank you @victorgcramos

Copy link
Member

@tiagoalvesdulce tiagoalvesdulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left a small comment.

return { record };
}

export function editRecordReply({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment wasn't addressed.

@tiagoalvesdulce tiagoalvesdulce merged commit 200dc2e into decred:master Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

e2e: Proposal Edit mock structure.
3 participants