Exercise 7 by powlaa · Pull Request #13 · Systems-Development-and-Frameworks/lichtow

powlaa · 2021-01-13T16:49:09Z

Review: Systems-Development-and-Frameworks/dojo#9 (review)

RespectableRuessel

⭐For instructions in the README.md on how to build your webapp for production.
(I think it's still the default README but it worked for us)
⭐ 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 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.
🔴 We didn't find software tests for the login / logout.

Looks good so far!

RespectableRuessel · 2021-01-25T15:01:28Z

webapp/store/index.js

+        state.token = token;
+        if (token) {
+            let { userId } = jwt_decode(token);
+            state.currentUser = userId;
+        } else {
+            state.currentUser = null;
+        }


Maybe consider to only set the token if it is valid. Otherwise (if the token contains garbage) your state will be broken because your loggedIn method checks !!state.token. So it will show True even if the token is invalid.

RespectableRuessel · 2021-01-25T15:04:11Z

webapp/components/NewsList/NewsList.spec.js

+        it("renders a message when the item list is empty", async () => {
+            createComponent({
+                allPostsQueryHandler: jest.fn().mockResolvedValue({ data: { posts: [] } }),
            });
+            await wrapper.vm.$nextTick();
+


⭐ Refactor looks good. We also used wrapper.vm.$nextTick() which is considered ugly but at least it works.

RespectableRuessel · 2021-01-25T15:07:21Z

webapp/components/NewsItem/NewsItem.vue

+      <button v-if="loggedIn" @click="upvote">Upvote</button>
+      <button v-if="loggedIn" @click="downvote">Downvote</button>
+      <button v-if="isAuthor" @click="remove">Remove</button>
+      <button v-if="isAuthor" @click="edit">Edit</button>


⭐Implemented Upvote and Downvote in logged in state, Remove and Edit only if it's the author. Optional: Only show the upvote or downvote button if you didn't upvoted / downvoted before.

➕ 1️⃣

jobegrabber

And

⭐ 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.

jobegrabber · 2021-01-26T22:16:03Z

webapp/README.md

+### build for production and launch server
+
+```
+npm run build
+npm run start
+```


Nice!

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

jobegrabber · 2021-01-26T22:18:53Z

webapp/components/LoginForm/LoginForm.spec.js

@@ -0,0 +1,75 @@
+import { createLocalVue, shallowMount } from "@vue/test-utils";


Good job!

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

jobegrabber · 2021-01-26T22:24:58Z

webapp/components/NavBar/NavBar.spec.js

@@ -0,0 +1,86 @@
+import { createLocalVue, shallowMount, RouterLinkStub } from "@vue/test-utils";


Looks great,

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

jobegrabber · 2021-01-26T22:28:34Z

webapp/components/NavBar/NavBar.spec.js

+        it("calls logout when logout button is clicked", () => {
+            const logoutBtn = wrapper.find("#logoutBtn");
+            logoutBtn.trigger("click");
+            expect(logout.mock.calls.length).toBe(1);


Suggested change

expect(logout.mock.calls.length).toBe(1);

expect(logout).toBeCalledTimes(1);

is the idiomatic way I think

jobegrabber · 2021-01-26T22:30:21Z

webapp/components/NewsItem/NewsItem.vue

+      <button v-if="loggedIn" @click="upvote">Upvote</button>
+      <button v-if="loggedIn" @click="downvote">Downvote</button>
+      <button v-if="isAuthor" @click="remove">Remove</button>
+      <button v-if="isAuthor" @click="edit">Edit</button>


➕ 1️⃣

jobegrabber · 2021-01-26T22:30:47Z

webapp/components/NewsItem/NewsItem.vue

-      <button @click="upvote">Upvote</button>
-      <button @click="downvote">Downvote</button>
-      <button @click="remove">Remove</button>
+      <button v-if="loggedIn" @click="upvote">Upvote</button>


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

medizinmensch · 2021-02-03T11:44:44Z

Hi @Systems-Development-and-Frameworks/lichtow !

I got issues with building your app. It looks like at least one .gql file is missing?

yarn generate
yarn run v1.22.10
$ nuxt generate

 WARN  When using nuxt generate, you should set target: 'static' in your nuxt.config                                                                                     👉 Learn more about it on https://go.nuxtjs.dev/static-target

ℹ️ Production build                                                                                                                                               
ℹ️ Bundling for server and client side                                                                                                     
ℹ️ Target: static                                                                                                                                                   
✔️ Builder initialized                                                                                                                                             
✔️ Nuxt files generated                                                                                                                                       

✖️ Client
  Compiled with some errors in 9.37s

◯ Server
  


Hash: cd71a9ba8f2eea713ed0
Version: webpack 4.46.0
Time: 9370ms
Built at: 02/03/2021 12:20:32 PM
     Asset      Size  Chunks               Chunk Names
0f0f11e.js  2.32 KiB       4  [immutable]  runtime
3bfc721.js  5.13 KiB       6  [immutable]  
65e1c16.js  56.2 KiB       0  [immutable]  app
7bcc1d3.js  2.63 KiB       3  [immutable]  pages/login
  LICENSES  1.35 KiB                       
ca9c59f.js   205 KiB       1  [immutable]  commons/app
d3c8c37.js   227 KiB       5  [immutable]  vendors/app
ee66a56.js  14.8 KiB       2  [immutable]  pages/index
Entrypoint app = 0f0f11e.js ca9c59f.js d3c8c37.js 65e1c16.js

ERROR in ./store/index.js
Module not found: Error: Can't resolve '../gql/login.gql' in '/home/youri/dev/uni/sdaf/groups/lichtow/webapp/store'
 @ ./store/index.js 3:0-41 45:26-31
 @ ./.nuxt/store.js
 @ ./.nuxt/index.js
 @ ./.nuxt/client.js
 @ multi ./node_modules/@nuxt/components/lib/installComponents.js ./.nuxt/client.js

 FATAL  Nuxt build error                            

  at WebpackBundler.webpackCompile (node_modules/@nuxt/webpack/dist/webpack.js:5497:21)
  at processTicksAndRejections (node:internal/process/task_queues:94:5)
  at async WebpackBundler.build (node_modules/@nuxt/webpack/dist/webpack.js:5446:5)
  at async Builder.build (node_modules/@nuxt/builder/dist/builder.js:5634:5)
  at async Generator.initiate (node_modules/@nuxt/generator/dist/generator.js:171:7)
  at async Generator.generate (node_modules/@nuxt/generator/dist/generator.js:133:5)
  at async Object.run (node_modules/@nuxt/cli/dist/cli-generate.js:376:24)
  at async NuxtCommand.run (node_modules/@nuxt/cli/dist/cli-index.js:2803:7)


   ╭─────────────────────────────╮
   │                             │
   │   ✖️ Nuxt Fatal Error        │
   │                             │
   │   Error: Nuxt build error   │
   │                             │
   ╰─────────────────────────────╯

Same is happening with npm dev .

Could you help me out here? Do you have the login.gql?

It looks good so far. Not sure why the file is missing.

Youri

powlaa · 2021-02-03T12:08:19Z

You're absolutely right, the file was missing. It was deleted accidentally in the last commit 😅
I added the file and it is working for me now, can you try again?

powlaa · 2021-02-03T12:28:50Z

The import was also messed up. Not sure why we didn't realize this before. So sorry!

roschaefer

Yay, you received all ⭐ in this exercise, 13 out of 13. Excellent work.

⭐ 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 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.

See a few suggestions below

roschaefer · 2021-02-11T20:15:57Z

webapp/components/LoginForm/LoginForm.spec.js

+        expect(submitBtn.exists()).toBe(true);
+    });
+
+    describe("invalid credentials", () => {


valid credentials would be nice.

roschaefer · 2021-02-11T20:17:48Z

webapp/components/LoginForm/LoginForm.vue

@@ -0,0 +1,64 @@
+<template>
+  <div>
+    <form onsubmit="event.preventDefault();">


Suggested change

<form onsubmit="event.preventDefault();">

<form @submit.prevent="submit">

roschaefer · 2021-02-11T20:19:16Z

webapp/components/LoginForm/LoginForm.vue

+        type="submit"
+        aria-label="Login"
+        value="Login"
+        @click="submit"


If you have the submit on the <form> you can omit that. With the form submit, you could also submit by hitting enter.

roschaefer · 2021-02-11T20:20:13Z

webapp/components/LoginForm/LoginForm.vue

+      }
+    },
+  },
+  data: function () {


Suggested change

data: function () {

data() {

roschaefer · 2021-02-11T20:20:40Z

webapp/components/LoginForm/LoginForm.vue

+export default {
+  methods: {
+    ...mapActions(["login"]),
+    submit: async function () {


Suggested change

submit: async function () {

async submit() {

roschaefer · 2021-02-11T20:40:32Z

webapp/gql/PostFragment.gql

@@ -0,0 +1,8 @@
+fragment Post on Post {


Very nice, you discovered fragments.

roschaefer · 2021-02-11T20:41:33Z

webapp/nuxt.config.js

+        authenticationType: "Bearer",
+
+        // Token name for the cookie which will be set in case of authentication
+        tokenName: "apollo-token",


It's the default, no?

Suggested change

tokenName: "apollo-token",

roschaefer · 2021-02-11T20:41:46Z

webapp/nuxt.config.js

+            },
+        },
+        // Sets the authentication type for any authorized request.
+        authenticationType: "Bearer",


Suggested change

authenticationType: "Bearer",

Default?

roschaefer · 2021-02-11T20:59:12Z

webapp/components/NavBar/NavBar.vue

+<style>
+</style>


Suggested change

<style>

</style>

roschaefer · 2021-02-11T21:00:46Z

webapp/components/NavBar/NavBar.vue

+  <div>
+    <template v-if="loggedIn">
+      <button id="logoutBtn" @click="logout">Logout</button>
+    </template>
+    <template v-else>
+      <NuxtLink to="/login">Login</NuxtLink>
+    </template>
+  </div>


Suggested change

<div>

<template v-if="loggedIn">

<button id="logoutBtn" @click="logout">Logout</button>

</template>

<template v-else>

<NuxtLink to="/login">Login</NuxtLink>

</template>

</div>

<div>

<button v-if="loggedIn" id="logoutBtn" @click="logout">Logout</button>

<NuxtLink v-else to="/login">Login</NuxtLink>

</div>

You could even remove the <div> if you want. An if-clause with an else is allowed as root node.

roschaefer · 2021-02-11T21:07:53Z

webapp/nuxt.config.js

+            background: "#ffffff",
+            theme_color: "#90EE90",
+        },
+    },


powlaa and others added 3 commits January 13, 2021 17:47

Add vue-apollo and graphql-tag

db8ca28

setup login button in webapp

91a4e36

Add Login functionality

6490100

powlaa force-pushed the exercise-7 branch from 13bf05e to 6490100 Compare January 20, 2021 15:10

powlaa added 9 commits January 20, 2021 18:53

WIP add login and logout

b65c69e

Add login redirect and on load

700d19d

Set User when Token set

bb2ac3b

Upvote, Downvote, Write, Delete

1bc5a13

Hide Buttons if not logged in or author

544b72c

Add edit button

8c6134e

Fix Tests

0fe127a

Fix lint errors

1285621

Remove test for authorization for posts query

271baf2

RespectableRuessel suggested changes Jan 25, 2021

View reviewed changes

RespectableRuessel mentioned this pull request Jan 25, 2021

Exercise 7 Systems-Development-and-Frameworks/no-js-pls#9

Open

powlaa added 6 commits January 26, 2021 12:25

Add NavBar component

05cf094

Add test for LoginForm

bb045a1

Configure PWA

e569b39

Add currentUser to loggedIn check

6f92197

Update instructions

a4ad086

Refactor queries and mutations

3d31d5a

powlaa requested review from jobegrabber, medizinmensch and roschaefer January 26, 2021 20:16

jobegrabber reviewed Jan 26, 2021

View reviewed changes

jobegrabber self-requested a review January 26, 2021 22:32

jobegrabber approved these changes Jan 26, 2021

View reviewed changes

jobegrabber mentioned this pull request Jan 30, 2021

Exercise7 Systems-Development-and-Frameworks/dojo#9

Open

Add missing login mutation

d680559

Fix import of login mutation

c21b280

roschaefer approved these changes Feb 11, 2021

View reviewed changes

roschaefer reviewed Feb 11, 2021

View reviewed changes

webapp/nuxt.config.js

background: "#ffffff",

theme_color: "#90EE90",

},

},

Copy link

Contributor

roschaefer Feb 11, 2021

Choose a reason for hiding this comment

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

		@@ -0,0 +1,75 @@
		import { createLocalVue, shallowMount } from "@vue/test-utils";

		@@ -0,0 +1,86 @@
		import { createLocalVue, shallowMount, RouterLinkStub } from "@vue/test-utils";

	expect(logout.mock.calls.length).toBe(1);
	expect(logout).toBeCalledTimes(1);

	<form onsubmit="event.preventDefault();">
	<form @submit.prevent="submit">

Conversation

powlaa commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RespectableRuessel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jobegrabber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

medizinmensch commented Feb 3, 2021

Uh oh!

powlaa commented Feb 3, 2021

Uh oh!

powlaa commented Feb 3, 2021

Uh oh!

roschaefer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

powlaa commented Jan 13, 2021 •

edited

Loading