Organization Campaign Page #55
Conversation
|
|
||
| it('contains a campaign title', () => { | ||
| cy.visit('/o/1/campaigns/941'); | ||
| cy.get('[data-test="campaign-heading"]').should('be.visible'); |
There was a problem hiding this comment.
This works, but it doesn't actually check that the title is being rendered, just that an element (which may or may not contain the title) with a certain ID is being rendered.
I would have preferred checking for the actual title from dummyCampaign.
src/fetching/getCampaignEvents.ts
Outdated
| interface ZetkinEvent { | ||
| activity: { title: string }, | ||
| campaign: { title: string }, | ||
| end_time: string, | ||
| id: number, | ||
| location: { title: string }, | ||
| start_time: string, | ||
| title: string | ||
| } |
There was a problem hiding this comment.
Wasn't this already implemented somewhere else? Instead of duplicating, I think we should split this out into a separate file and reuse the same interface in both places. Does that make sense?
There was a problem hiding this comment.
Yes, it is also used in getEvents. Sounds good!
| it('contains campaign information', () => { | ||
| cy.visit('/o/1/campaigns/941'); | ||
| cy.get('[data-test="campaign-information"]').should('be.visible'); | ||
| }); |
There was a problem hiding this comment.
I don't think this should need to be a separate test from the previous one. Just have one test to assert that all campaign information is being displayed, both the title and the description in the same it block.
There was a problem hiding this comment.
I agree. Is it a problem that this doesn't check the content just the element, as with the title?
There was a problem hiding this comment.
I would say that the issue is the same here. With a small difference being that you could envision a scenario where the text is so long that it is truncated (which would cause the test to fail if you're looking for the entire text). But for the test case the easiest way to solve that is the make sure that the dummy text isn't that long.
There was a problem hiding this comment.
Some more comments below!
It turns out the linter has not been running on the cypress directory, and hence has not been checking the integration test code. These are the errors it found in the org_campaign_page.spec.ts file relevant to this PR:
/Users/richardolsson/src/zetkin/product/app.zetkin.org/cypress/integration/org_campaign_page.spec.ts
3:1 error Expected indentation of 4 spaces but found 8 indent
4:1 error Expected indentation of 8 spaces but found 12 indent
11:15 error Missing semicolon semi
12:1 error Expected indentation of 4 spaces but found 8 indent
14:1 error Expected indentation of 4 spaces but found 8 indent
15:1 error Expected indentation of 8 spaces but found 12 indent
17:1 error Expected indentation of 8 spaces but found 12 indent
18:1 error Expected indentation of 8 spaces but found 12 indent
19:1 error Expected indentation of 4 spaces but found 8 indent
22:1 error Expected blank line before this statement padding-line-between-statements
I suggest you fix these now, and then we'll have to fix the rest in a separate PR.
Run eslint cypress to run the linter on all JS/TS files in the cypress directory.
cypress/fixtures/dummyCampaign.json
Outdated
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "data": { | |||
| "id": 1, | |||
There was a problem hiding this comment.
Indentation looks off in this file.
src/fetching/getCampaign.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| export default function getCampaign(orgId : string, campId : string) { | |||
| return async () : Promise<{ title: string }> => { | |||
| return async () : Promise<{ title: string, info_text: string }> => { | |||
There was a problem hiding this comment.
Maybe introduce a type/interface for this as well? E.g. ZetkinCampaign.
I've tried this is the terminal with no luck? |
Fixes #41
Adjustments to organization campaign page. Contains campaign title and information as well as events of the campaign.