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

Add create new project #142

Merged
merged 23 commits into from
Apr 24, 2019

Conversation

aonomike
Copy link
Contributor

@aonomike aonomike commented Mar 9, 2019

  • Add cypress tests for the create new project
  • Add components for CreateProjectPage
  • Add route for 'projects/new'

Issue addressed

fixes #139

Screenshots (if appropriate):

Testing

@ghost ghost assigned aonomike Mar 9, 2019
@ghost ghost added the review label Mar 9, 2019
@vercel
Copy link

vercel bot commented Mar 9, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://websiteone-fe-git-fork-aonomike-139createnewprojects.agileventures1.now.sh

@aonomike aonomike changed the title Add create new project [WIP] Add create new project Mar 9, 2019
@joaopapereira joaopapereira added this to In progress in WebsiteOne Mar 26, 2019
@mattwr18 mattwr18 mentioned this pull request Apr 4, 2019
@aonomike aonomike changed the title [WIP] Add create new project Add create new project Apr 17, 2019
WebsiteOne automation moved this from In progress to Review in progress Apr 18, 2019
Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

Looks good, please review my comments

<option value='Active' >Active</option>
<option value='Inactive'>Inactive</option>
</select>
<Button type='submit' > Submit </Button> </Form >
Copy link
Member

Choose a reason for hiding this comment

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

What do you think we if we call this Create Project

expect(wrapper.find('ProjectForm')).toHaveLength(1)
})

// it('calls handleSubmit when the form is submitted', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this test commented out?

.get('textarea[name=description]')
.type('A new project')
.get('select[name=status]')
.get('button[type=submit]')
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I just noticed this @aonomike, why are we selecting the status? we aren't doing anything with it

cy.fixture('newlyCreatedProjectInfo').then(newlyCreatedProjectInfo => {
cy.route(/\/api\/v1\/projects\/newproject/, newlyCreatedProjectInfo).as('getProject')
cy.visit('/projects/newproject')
cy.window()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to change this cypress test to do what we are doing with the create events functionality in the React Mob.

You rightly brought up the bug that the way I had written the test was actually not testing that a user is being redirected by a successfully created event/project because I was, and you are, visiting the newlyCreatedProject.

import initialState from './initialState'

const createProjectReducer = (state = initialState.project, action) => {
switch (action.type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As I brought up to you @aonomike, I am having doubts about what the point of saving the newlyCreatedProject/Event to the store when we are redirecting the user to the newlyCreatedProject/Event's info page where it reaches out to the backend to grab the project's/event's info.

maybe it's a refactor? @joaopapereira what do you think? I can't think of any reason we need to have the user carry around the newlyCreatedProject/Event's info since the info they request from the backend on componentDidMount on the ProjectInfo.js/EventInfo.js includes all of the project's/event's info plus other info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the Stephen Grider course, he also does dispatch the response after create to store, maybe its just a convention? maybe there is something to it. Your thoughts @Kachulio1 @arku @FedericoEsparza @joaopapereira ?

})

it('calls method to push the user to the events info page', () => {
moxios.wait(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

really like that you had the idea to test for this, especially since we need to pass in a mock functionf for history.push, we might as well test that it's being called 😸
good catch... need to create refactoring tickets to borrow your ideas 😸

describe('CreateProjectPage', () => {
let wrapper
let props = {
history: { push: jest.fn() },
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this, or was this added when we were thinking of handling the redirection from the CreateProjectPage.js?

- Add cypress tests for the create new project
- Add components for CreateProjectPage
- Add route for 'projects/new'
WebsiteOne automation moved this from Review in progress to Reviewer approved Apr 24, 2019
@joaopapereira joaopapereira merged commit 12b59c3 into AgileVentures:develop Apr 24, 2019
WebsiteOne automation moved this from Reviewer approved to Done Apr 24, 2019
@ghost ghost added production and removed review labels Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
WebsiteOne
  
Done
Development

Successfully merging this pull request may close these issues.

Create new projects
4 participants