Skip to content

Exercise 7 - #47

Merged
aloparev merged 28 commits into
mainfrom
exercise-7
Feb 19, 2021
Merged

Exercise 7#47
aloparev merged 28 commits into
mainfrom
exercise-7

Conversation

@yiimnta

@yiimnta yiimnta commented Jan 18, 2021

Copy link
Copy Markdown
Collaborator

@yiimnta
yiimnta requested review from aloparev and ilonae January 18, 2021 20:11
@yiimnta
yiimnta requested review from MaykAkifovski and ubiquitousbyte and removed request for MaykAkifovski January 23, 2021 10:30

@MaykAkifovski MaykAkifovski left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You did a good job.

  • You need to finish the tests.
  • I think I didn't see any changes from refactoring vue-cli to create-nuxt-app. ⭐ from me

Comment on lines +1 to +125
<template>
<form class="login-form" @submit.prevent="submit">
<div v-if="error">
<small class="error-text"> {{ error.message }}</small>
</div>
<div v-if="loading">
<small class="loading-text">Loading...</small>
</div>
<input
id="email"
v-model.trim="formData.email"
name="email"
type="email"
placeholder="Email"
/>
<input
id="password"
v-model.trim="formData.password"
name="password"
type="password"
placeholder="Password"
/>
<button class="login-btn" type="submit" :disabled="loading || !valid">
Login
</button>
<div>
<small>Not a member?</small>
<NuxtLink class="register-btn" to="signup"> Sign up now </NuxtLink>
</div>
</form>
</template>
<script>
import { mapActions } from 'vuex'
import { UNKNOWN_ERROR, LOGIN_ERRORS } from '@/static/error'

export default {
name: 'LoginForm',
data() {
return {
formData: {
email: '',
password: '',
},
error: null,
loading: false,
}
},
computed: {
valid() {
const { email, password } = this.formData
return email && password
},
},
methods: {
...mapActions('auth', ['login']),
async submit() {
try {
this.error = null
this.loading = true
await this.login({ ...this.formData })
this.$router.push({ path: '/' })
} catch (ex) {
let message = ex.message.replace('GraphQL error:', ' ').trim()
if (!LOGIN_ERRORS.includes(message)) {
message = UNKNOWN_ERROR
}
this.error = { message }
} finally {
this.loading = false
}
},
},
}
</script>
<style scoped>
.login-form {
display: grid;
width: 100%;
}

.login-form input {
height: 35px;
border-radius: 5px;
border: 1px solid;
background: aliceblue;
padding: 0 10px;
margin: 10px 0px;
outline: none;
}

.error-text,
.loading-text {
text-align: left;
padding: 5px 0px 10px;
font-weight: 900;
}

.error-text {
color: #f50749;
}

.loading-text {
color: green;
}

.login-btn {
height: 35px;
margin-bottom: 10px;
background: #75c2f9;
color: black;
border: none;
border-radius: 5px;
}

.login-btn:disabled {
background: darkgray;
cursor: no-drop;
outline: none;
}

.register-btn {
text-decoration: none;
color: #0d9ef3;
}
</style>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐ for implementing a Login Form

Comment thread webapp/components/News/News.vue Outdated
Comment on lines +6 to +7
<button @click="upvote" v-if="isAuthenticated">Upvote</button>
<button @click="downvote" v-if="isAuthenticated">Downvote</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐ For implementing upvote and downvote and binding them with the authentication

Comment thread webapp/nuxt.config.js
Comment on lines +50 to +58
apollo: {
// Sets up the apollo client endpoints
clientConfigs: {
default: {
// required
httpEndpoint: 'http://localhost:4000',
},
},
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐ for implementing an apollo connection

Comment thread webapp/store/auth.js
Comment on lines +31 to +38
async login({ commit }, { email, password }) {
const res = await this.app.apolloProvider.defaultClient.mutate({
mutation: LOGIN,
variables: { email, password },
})

const token = res.data.login
await commit(SET_TOKEN, token)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐ for implementing login

Comment on lines +1 to +93
import { shallowMount, createLocalVue } from '@vue/test-utils'
import Vuex from 'vuex'
import { UNKNOWN_ERROR, EMAIL_EXIST, PASSWORT_SHORT } from '@/static/error'
import { GraphQLError } from 'graphql'
import SignupForm from './SignupForm.vue'

const localVue = createLocalVue()
localVue.use(Vuex)

describe('LoginForm.vue', () => {
let actions
let getters
let store

const setupWrapper = () => {
store = new Vuex.Store({
modules: {
auth: {
namespaced: true,
state: () => ({
currentUser: null,
token: null,
}),
actions,
getters,
},
},
})
const stubs = { NuxtLink: true }
return shallowMount(SignupForm, { store, localVue, stubs })
}

beforeEach(() => {
getters = {
isAuthenticated: () => false,
}
actions = {
signup: jest.fn(),
}
})

describe('form submit', () => {
const signup = async (wrapper) => {
wrapper.find('input#name').setValue('Test User')
wrapper.find('input#email').setValue('testuser@gmail.com')
wrapper.find('input#password').setValue('12345678')
await wrapper.find('form').trigger('submit')
}

it('shows no error', async () => {
const wrapper = setupWrapper()
await wrapper.find('form').trigger('submit')
expect(wrapper.find('.error-text').exists()).toBe(false)
})

describe('when register are wrong', () => {
it('shows password short error', async () => {
actions.signup = jest
.fn()
.mockRejectedValue(new GraphQLError(PASSWORT_SHORT))
const wrapper = setupWrapper()
await signup(wrapper)
await localVue.nextTick()
expect(wrapper.find('.error-text').text()).toContain(PASSWORT_SHORT)
})

it('shows email exist error', async () => {
actions.signup = jest
.fn()
.mockRejectedValue(new GraphQLError(EMAIL_EXIST))
const wrapper = setupWrapper()
await signup(wrapper)
await localVue.nextTick()
expect(wrapper.find('.error-text').text()).toContain(EMAIL_EXIST)
})
})

describe('in case of any other error', () => {
beforeEach(() => {
actions.signup = jest
.fn()
.mockRejectedValue(new Error('Any other Error'))
})

it('shows wrong credentitals error', async () => {
const wrapper = setupWrapper()
await signup(wrapper)
await localVue.nextTick()
expect(wrapper.find('.error-text').text()).toContain(UNKNOWN_ERROR)
})
})
})
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐ for testing

Comment thread webapp/README.md
Comment on lines 12 to 15
# build for production and launch server
$ yarn generate
$ yarn build
$ yarn start

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐ for extending the Readme

@yiimnta
yiimnta requested review from ubiquitousbyte and removed request for ubiquitousbyte January 28, 2021 01:18

@roschaefer roschaefer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good job

I just have to deduce one ⭐ for the broken static deployment, see below 👇

I had to manually remove the target: 'static' in .nuxt.config.js and run

yarn run build
yarn run start

Apart from that, well done @Systems-Development-and-Frameworks/countryroads! You receive 12/13 ⭐ in this exercise.

❌ For instructions in the README.md on how to build your webapp for production.

⭐ For no changes in "Files Changed" tab of the refactoring from vue-cli to create-nuxt-app. (See #1 in instructions)

⭐ ⭐ For the API connection between your front- and backend.

⭐ For your previous frontend tests still passing. Requests to the backend are mocked.

⭐ ⭐ For a login feature in your webapp including a Vue component and its software tests.

⭐ ⭐ For a menu component which shows a login or logout button and its software tests.

⭐ For an upvote button that behaves according to the authentication state of your user

⭐ For a delete and edit button that is only visible to the author of the post.

⭐ For Lighthouse reporting that your production website is installable as PWA (except HTTPS).

⭐ For requesting a review and reviewing another team's PR.

Comment thread webapp/README.md

# generate static project
$ yarn generate
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❌ For instructions in the README.md on how to build your webapp for production.

The build is broken, when I follow the instructions.

Comment thread exercises/6/README.md
'` \)_`"""""`
.--' ')
o( )_-\
`"""` `

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are the exercise descriptions in the README.md 😢 ? Please rebase your changes or merge homework/main.

Comment thread backend/src/utils.js

if(voteType === NOT_ALLOWED_VOTE) {
throw new UserInputError("This user voted on that post already.");
if(voteType === NOT_ALLOWED_UPVOTE) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the voteType is unnecessary state and your nested if-clause is very complex and hard to read. Try to use guard-clauses and throw an error in case. The execution will stop if an error occurs.

Comment thread webapp/README.md

# serve with hot reload at localhost:3000
$ yarn dev
$ yarn dev (all PWA functions are not supported)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤔 ?

Comment on lines +51 to +52
wrapper.find('input').setValue('')
await Vue.nextTick()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
wrapper.find('input').setValue('')
await Vue.nextTick()
await wrapper.find('input').setValue('')


describe('Delete', () => {
const remove = async () => {
news.authored = true //to show the delete button

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⭐ For a delete and edit button that is only visible to the author of the post.

id
title
votes
authored

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Group @Systems-Development-and-Frameworks/lichtow has used GraphQL fragments here to reduce duplication: https://graphql.org/learn/queries/#fragments

Comment thread webapp/nuxt.config.js
name: 'CountryRoads News App',
lang: 'en',
start_url: '/',
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⭐ For Lighthouse reporting that your production website is installable as PWA (except HTTPS).

Swappshot Fri Feb 12 15:37:06 2021

Comment thread webapp/README.md
Comment on lines +13 to 14
$ yarn generate
$ yarn build

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Depending on your deployment target you have either yarn run build or yarn run generate.

Comment thread webapp/store/index.js

export const actions = {
async nuxtServerInit(store, { ssrContext }) {
const { cookie } = ssrContext.req.headers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When I run yarn run generate this breaks because:

 ERROR   /404                                                                                                                                                                                             15:27:46

TypeError: Cannot read property 'headers' of undefined
    at Store.nuxtServerInit (store/index.js:7:0)
    at Array.wrappedActionHandler (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:853:23)
    at Store.dispatch (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:518:15)
    at Store.boundDispatch [as dispatch] (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:408:21)
    at module.exports.__webpack_exports__.default (node_modules/.cache/nuxt/server.js:135:0)
    at runNextTicks (internal/process/task_queues.js:58:5)
    at listOnTimeout (internal/timers.js:523:9)
    at processTimers (internal/timers.js:497:7)


 ERROR   /login                                                                                                                                                                                           15:27:46

TypeError: Cannot read property 'headers' of undefined
    at Store.nuxtServerInit (store/index.js:7:0)
    at Array.wrappedActionHandler (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:853:23)
    at Store.dispatch (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:518:15)
    at Store.boundDispatch [as dispatch] (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:408:21)
    at module.exports.__webpack_exports__.default (node_modules/.cache/nuxt/server.js:135:0)
    at runNextTicks (internal/process/task_queues.js:58:5)
    at listOnTimeout (internal/timers.js:523:9)
    at processTimers (internal/timers.js:497:7)


 ERROR   /signup                                                                                                                                                                                          15:27:46

TypeError: Cannot read property 'headers' of undefined
    at Store.nuxtServerInit (store/index.js:7:0)
    at Array.wrappedActionHandler (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:853:23)
    at Store.dispatch (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:518:15)
    at Store.boundDispatch [as dispatch] (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:408:21)
    at module.exports.__webpack_exports__.default (node_modules/.cache/nuxt/server.js:135:0)
    at runNextTicks (internal/process/task_queues.js:58:5)
    at listOnTimeout (internal/timers.js:523:9)
    at processTimers (internal/timers.js:497:7)


 ERROR   /                                                                                                                                                                                                15:27:46

TypeError: Cannot read property 'headers' of undefined
    at Store.nuxtServerInit (store/index.js:7:0)
    at Array.wrappedActionHandler (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:853:23)
    at Store.dispatch (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:518:15)
    at Store.boundDispatch [as dispatch] (/home/robert/Development/Systems-Development-and-Frameworks/homework/node_modules/vuex/dist/vuex.common.js:408:21)
    at module.exports.__webpack_exports__.default (node_modules/.cache/nuxt/server.js:135:0)

Your ssrContext does not have a request when you build a static deployment. Makes sense, no?

@aloparev
aloparev marked this pull request as ready for review February 19, 2021 12:14
@aloparev
aloparev merged commit 13cd08c into main Feb 19, 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.

5 participants