facn3 / his_node Goto Github PK

listen is an asynchronous function so you're logging, so it can take a callback, the right way would be to log from that callback once it executes

Adding a process pic for readme.

sending an API request from the modal.js file to the wiki API.
getting the response and handle it.

const router = require('./router');

var app = function (request, response) {
    router(request, response);
};

http.createServer(app).listen(port);

Is functionally the same as:

const router = require('./router');

http.createServer(router).listen(port)

The project spec is to autocomplete to the front end from a large file of words. I'm glad that you went out of your comfort zone and used the request module to hit the wiki api, but that should have been in a different section of the app. For example after the autocomplete you could have hit search and been given a wiki result. The project spec says:

A large data file is required to search through - consider the best data structure for this (e.g. .txt or .json).

Currently that's not happening.

creating testing files for the server side.

creating testing files for client side.

The Image of software architecture shows 404.
and need to be fixed.

Apparently the auto complete words are always starting with 'O'

and it only shows the same 10 words: 😓

• Obama
• Obama, Fukui
• Obama Doctrine
• Obama (genus)
• Obama Anak Menteng
• Obama nungara
• Obama logo
• Obama Domain
• Obamadon
• Obama Zombies

Looks like all the issues your project had have been raised. I am impressed by the amount of work you guys have put into the project. Keep it up.

At the moment there's no way to run the site without cloning the site and running locally. It's on the the learning outcomes for the week so I'd strongly encourage sorting it within the time for responding to code review issues.

Something like the function below can be used to clear old results when a new autocomplete comes in:

function removeChildren(element) {
  while (element.hasChildNodes()) {
    element.removeChild(element.lastChild);
  }
}

handle data from API request in logic.js as well as html and css

It's common practice to have both a start script and some sort of dev script in your package.json. You'll need one to host on Heroku (#37) and also it's generally the first thing people try when they clone a node project -- devStart should be for developing, not running the project.

https://github.com/FACN3/HIS_node/blob/master/src/model.js#L2
you're getting searchQuery as an input but then changing it in the first line with some mock data, a better practice would be to put any mock data you need in a new file and exporting that file.

The only thing I'd change is that your accessing the object multiple times. It would be better to do this once, and within the function, and you could also add destructuring for nicer naming. Something like:

const {staticFiles, model, html, notFound} = require('./handlers');

module.exports = function(request, response) {

    const route = {
        '/' : html,
        '/style.css' : staticFiles,
        '/logic.js'  : staticFiles,
        '/model.js' : model,
    }[request.url]


    if (route) {
       route(request, response);
    } else {
       notFound(request, response);
    }
}

I think you should remove all the extra codes (comments and console.logs)

i.e.
index.html -- line 15
<!-- <img class="groupePhoto" src="" alt="groupe photo"> -->

data.js -- line 1 - 5

// <script src="http://en.wikipedia.org/w/api.php?action=opensearch&limit=10&format=json&callback=getData&search="></script>
//
//
// <input type="text" id="searchbox">
// <div id="output"></div>
//this part will go to html file later

starts working on api

The html function could be massively simplified by changing staticFile to take a url as a third argument and then calling it within html, something like this:

handlers.html = function (request, response) {
  handlers.staticFiles(request, response, '/index.html');
}

handlers.staticFiles = function (request, response, url) {
    const extension = url.split('.')[1];
    const extensionType = {
        'html': 'text/html',
        'css': 'text/css',
        'js': 'javascript/application'
    }[extension];

    fs.readFile(__dirname + '/../public/'+ url,
    function (error, file) {
        if (error) {
            console.log(request.url);
            console.log('oh there is an error', error);
            return;
        }
        response.writeHead(200, { 'Content-Type': extensionType });
        response.end(file);
    });
}

vs

handlers.html = function (request, response) {
    const endpoint = request.url;
    if (endpoint === '/') {
        fs.readFile(__dirname + '/../public/index.html',
            function (error, file) {
                if (error) {
                    console.log(request.url);
                    console.log('oh there is an error', error);
                    return;
                }
                response.writeHead(200, { 'Content-Type': 'text/html' });
                response.end(file);
            });
     }
}

handlers.html = function (request, response) {
    const endpoint = request.url;
    if (endpoint === '/') {
        fs.readFile(__dirname + '/../public/index.html',
            function (error, file) {
                if (error) {
                    console.log(request.url);
                    console.log('oh there is an error', error);
                    return;
                }
                response.writeHead(200, { 'Content-Type': 'text/html' });
                response.end(file);
            });
     }
}

Creating an API request with search query form logic.js (client) to modal.js (server)

creating css, html, js files for the client side. #1

heroku prep_fixing codes

• port ->PORT
• added start to script

In model.js you have a for loop that could be replaced with something like this if you wanted (die for loops die):

const result = Array.from({length:10}, (el, index) => {
  return {
    title: parsedData[1][index],
    link: parsedData[3][index]
  };
});

I usually only recommend for loops when break is necessary.

playing around with wikipedia api so we can use it in the server side.

Issues
Awesome, lots of issues with descriptions. I would say a lot of the issues are very small and some could be joined together.

Commits
I ❤️ seeing almost all of your commits linked to issues. They mostly all have descriptive messages so I know what you did which is great 🥇.

Pull Requests
You guys are linking your pull requests to issues 👍. Just a minor point on this, you can't link to issues in the title of the PR so in future you only need it in the description rather than the title. PRs should be reviewed by your team members before you merge. You should assign your team as reviewers where they can approve ✅ , request changes ❌ or just ask questions ❓ about specific parts of the code they don't understand. The final reviewer should merge the PR. I understand in a small team like this it may seem like a waste of time but it is a good way to ensure the whole team understands the code.

Contributions
It would be great to see a more equal spread of contributions between team members on the graph 📈 . Keep swapping who is typing and the laptop you are working on throughout the project ⭐.

Keep up the awesome git flow ✨

Shouldn't there be a submit button to execute the search input? 🤔

The webpage dont seem to be responsive 😓

the image planing image is in the general folder.
we should move it to a resources folder.

The readme looks great 💯 A few minor notes though:

In the installation instructions you have some markdown within backticks ## like this. I'm guessing that was just a mistake.

There's also this line in the readme:

https://github.com/FACN3/HIS_node (needs to be changed)

Which should probably just be removed. You explain how to use the project below including cloning the repo so there isn't really a need for this.

to do - figure out why this is not working

creating skeleton files for the server side including :
.gitignore
handlers.js
model.js
package.json
router.js
server.js

creating a data file from which the data for the auto complete will come.

Re fixing pics on the readme

Test coverage with Istanbul

signing up for heroku for deploying the future app.

It's a vscode specific file that isn't necessary to run the project so should probably be in the .gitignore. The master branch should only contain files necessary to run the project.

creating a ReadMe.md file that will include:
how to clone, install, run.
explanation about the project.
the planing process.
and other nice things.

I think you should give some colors to the webpage
make it more "lovely" 😝

In handlers.model you handle file read errors:

        if (error){
            response.writeHead(404, { 'Content-Type': 'text/html' });
            response.end('error geting data from wiki', error);   
        }
        response.writeHead(200, { 'Content-Type': 'application/json' });
        response.end(JSON.stringify(purpose));

But none of your other handlers do this. Every branch a request can take should have a response.end so the request never hangs.

Changed the name of folder from 'resources' -> 'assets'
(since we have a folder called src, it is bit confusing :p )

what is playgrond?
if it's a file that you only need for local trials then you can ignore it in the .gitignore

You have an assets folder with images that are readme specific. These should be hosted in issues rather than the repository to to stop users who want to clone your project from having to download them.

Happy to show you the process for this 👍

i think the test aren't really testing anything? 😅

by my understanding it only checks if the tape is working?

but what about the server/client functionality 😓