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

Landing page #11

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

emma-r-slight
Copy link

HI this isn't finished (obviously) just thought i'd push this up so you can see what i've been doing, i have refactored some of the file names. This is to take advantage of how index.js works in directories, also so you don't have lines and lines of imports/requires from a directory. I also separated concerns out of the app/index start file for the server, so all it does is set the local host.

I'm using Bootstrap 4 for css if thats alright, happy to use something else if you have a preferred CSS framework.

const mongoose = require('mongoose')
Copy link
Owner

Choose a reason for hiding this comment

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

  • I'm setting the db URL here as a constant but i was planning to make it as environment variable later
  • I'm using express-async-errors package to handle errors like this one here
  • You used callback function to handle the connection result but the error is already handled by the error middleware and everything is logged using Winston package

Copy link
Author

@emma-r-slight emma-r-slight Oct 22, 2020

Choose a reason for hiding this comment

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

I understand what you are saying, but for some reason, this is not connecting to the db. The winston logging is great, and very handy!! I'll see if i can resolve this problem... any suggestions to incorporate winston? (the change i made to this file connected to mongodb)

Copy link
Owner

Choose a reason for hiding this comment

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

honestly I don't get you, can you elaborate for me

Copy link
Author

Choose a reason for hiding this comment

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

previously the way the file was set up there was no connection being made to the local mongoDB. The way i had rewritten the file connected to the local database so data was being added and could be retrieved from the mongoDB file.
I have now changed the code back to how you had it, removing the export function and adding "useCreateIndex: true".

The connection is now working to mongodb and a message is being added to the logfile.

@@ -0,0 +1,30 @@
const express = require('express')
Copy link
Owner

Choose a reason for hiding this comment

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

  • I have exactly the same code but in routes.js why you changed the file name ?

Copy link
Author

Choose a reason for hiding this comment

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

using index.js inside a directory means that you only need to require the directory name when importing the module.

eg: const app = require('../startup')

instead of

const app = require("../startup/routes")

Copy link
Owner

Choose a reason for hiding this comment

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

I figured this out and found it quite useful thank you

@ahmeddrawy
Copy link
Owner

the app crashes with me giving

internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module './website/login'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/tle23/side-projects/nodejs/rest-api/routes/index.js:5:15)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/tle23/side-projects/nodejs/rest-api/startup/index.js:4:57)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
[nodemon] app crashed - waiting for file changes before starting

startup/db.js Outdated Show resolved Hide resolved
@ahmeddrawy ahmeddrawy linked an issue Oct 20, 2020 that may be closed by this pull request
2 tasks
@ahmeddrawy ahmeddrawy added hacktoberfest help wanted Extra attention is needed labels Oct 20, 2020
@ahmeddrawy
Copy link
Owner

the app crashes with me giving

internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module './website/login'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/tle23/side-projects/nodejs/rest-api/routes/index.js:5:15)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/tle23/side-projects/nodejs/rest-api/startup/index.js:4:57)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
[nodemon] app crashed - waiting for file changes before starting

resolved by fixing routes/index.js requires

@emma-r-slight
Copy link
Author

Hi,
localhost:8080/ - will render the top nav bar and the hero banner

localhost:8080/api/courses/seed = will seed the database with courses

localhost:8080/api/courses - will render with the courses listed

Do you want to ba able to see the courses only after you have logged in? Or do you want to see them on the home page and only be able to add new courses or edit courses once you are logged in??

@@ -19,11 +20,16 @@ const schema = new mongoose.Schema({
},
},
})
module.exports = mongoose.model('Course', schema)

Copy link
Owner

Choose a reason for hiding this comment

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

I don't get the point why you write the same code I've written before in a different style ?

function validateCourse(course) {
const schema = Joi.object({
name: Joi.string().min(3).required(),
tags: Joi.array(),
})
return schema.validate(course)
}

exports.validate = validateCourse
Copy link
Owner

Choose a reason for hiding this comment

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

is this a kind of easier way or just a typo ?

@ahmeddrawy
Copy link
Owner

Hi,
localhost:8080/ - will render the top nav bar and the hero banner
It doesn't load anything

localhost:8080/api/courses - will render with the courses listed

Do you want to ba able to see the courses only after you have logged in? Or do you want to see them on the home page and only be able to add new courses or edit courses once you are logged in??

image

  • This is what I get and all buttons doesn't work ? does it work with you or should I fix it myself ?

localhost:8080/api/courses/seed = will seed the database with courses

  • This - I don't know why - crashes the app

  • should I implement the login logic by saving the token to cookies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make a home page to be redirected to after login
2 participants