Decision log

[berryjen]

I began this mini music API project to further solidify programming concepts learned thus far. I was able to set up the repoindependently and came to realize that the sequence in which you set up repo onGitHub made a difference. I was able to initiate the lead as well as in terms of the feature implementations as well as practice asking for assistance within a professional environment from Bill, where after attempting to debug myself, I then explained my logic to Bill and sought help from there.

1. create folder first, with only dashes allowed as spaces in between characters.
2. create newrepo on GH 3.install npmand following the prompts displayed on command line. I had to follow a beginner friendly node set up blog to further understand what to do with the prompts.

I referenced our travel API for the code & testing logic. I have learned that path isn't case sensitive ex) /music OR/Music as it is a fixed path; however, when the path involves parameters, then it is case sensitive because it is the comparison between data from the database and what the user provides that matters, so if it is not an exact match, it will result in error. This is also the case with the actual code on app.js.

I also attempted to write tests on my own. Errors started to appear when I ran my test, which displayed "module not found". Thus, I took the following steps to debug:

1. I compared the package.json file from travel API with the one from music API, detected missingdevDependencies, and copied & pasted onto the music API.
2. This still resulted in the same error so I reached out for assistance. Turned out that I need to (again) re-install, individually, npm onto the new project in order for the update to take place. There are 2 ways to install new modules:

• type in specifically in the terminal npm install --save-dev jest as this will only install jest. No need to re-install npm. OR
• manually include any missing dependencies in package.json file, re-run npm to see the changes come into effect.
• Supertest is entirely different fromjest, so that needs to be manually included.

I have also independently written a JSON object file which is populated with a list of 10 hard-coded arits, songs & genre to begin API testing.

Upon writing tests, I've learned that response.body is used to test JSONresponse & response.text is used when the body contains texts instead of JSON.

Bill also did a demo on code coverage, which is a developer tool on VSC that shows how much of the code is actually being tested by highlighting the parts that the tests cover.

We re-grouped the current test I wrote into 2 separate, more descriptive wording where one entails the status code, and the other text message.

describe('GET /music/:artist with an invalid artist', () => {
    test('should respond with 404', async () => {
        const response = await request(app).get('/music/321');
        expect(response.statusCode).toBe(404);
    });
    test('should respond Artist not found', async () => {
        const response = await request(app).get('/music/Disclosure');
        expect(response.text).toBe('Artist not found');
    });
});

To conclude this music API, I will continue to write coverage for a few more tests, with a focus on language consistency (the describe part) and keeping the test structure identical (describe, test, expect). Work on app.js to ensure the code matahces the tests. Then, I will incorporate Deezer as the external API into my database.

[berryjen]

I had been working on writing tests to make them pass, where one particular test was a bit more challenging.

describe('GET /music/:artist', () => {
    test('should respond with correct Artist ', async () => {
        const response = await request(app).get('/music/Queen');
        expect(response.obj.Artist === 'string').toBe(true);
    });

I over-complicated my logic and thought string would be an all-capturing net for the artists as we wouldn't know in advance the values the users would provide. Bill hinted that, first of all, Artist wasn't a property of obj as it didn't exist(although you do get back an object but Artist didn't exist in obj) secondly, the value we are comparing with is Queen; there wasn't an artist with the name string. So the correct test now looks like:

describe('GET /music/:artist', () => {
    test('should respond with correct Artist ', async () => {
        const response = await request(app).get('/music/Queen');
        expect(response.body.Artist === 'Queen').toBe(true);
    });

I also had to update the code that would reflect the new tests. I also had a bit of difficulty writing the logic myself initially. When I placed console.log(result) before return ``` obj.Artist === req.params.artist; });

it was logging undefined for all of the artists as results hadn't yet been created. I knew then it had to be before `res.json(result);` and it turned out to be correct. It was logging artists and returning undefined for invalid artists. At this point, the if conditions would be created around this. 

if (result !== undefined) { console.log(result); res.json(result); } else { res.status(404).send('Artist not found'); } });

Next, we tackled the new feature of incorporating Deezer that would allow users to get a recommended list of 10 new randomized songs whenever Deezer updates the list. After researching on Deezer, we have decided that `https://developers.deezer.com/api/explorer?url=chart` will allow us to do that. We discussed how we were going to route the requests among user, API & deezer. There were 2 scenarios: 
a) going from user request to API, then API to Deezer
Pros:

- always latest data(song)

Cons:

- takes longer time to get response back as server may be located at a location far way from the API request
- if the user overloads 3rd party server with API requests, it may overload server website, causing it to shutdown
- there may be a limit on traffic requests
- more expensive to maintain
- more complicated, nested functions

b) going from API to Deezer on a scheduled basis retrieving the updated list, and then returning the request from API to user
Pros:
- quicker response time
- no limit on # of user API requests 
- reduces infrastructure cost
- cleaner code

Cons:
- not the latest data (song)

We chose option B as this is the way the code has been structured thus far; storing `data.json `in a variable in `app.js` to make it accessible. 
Bill has shown a more efficient way of making the Deezer API call using Postman. We typed the web address, and there was a button next to the address bar that provided written code according to the language of one's choosing. We went with `Node.js-Axios` as I have previously worked with it on my React weather projects and the code involved using `Promise` chains, a concept that we had been learning for the past month. 
We had to download the Axios library and replace the code for incorporating `data.json` with the API code instead. I then subsequently updated the code in `app.js` to reflect the properties returned from the new API. 

app.get('/music/:artist', (req, res) => { var result = melody.tracks.data.find((track) => { return track.artist.name === req.params.artist; });

app.get('/music/:artist/:song', (req, res) => { var result = melody.tracks.data.find((track) => { return track.title === req.params.song && track.artist.name === req.params.artist; });

The last remaining work is to rewrite the tests to reflect the new working code and get them to pass. 

[berryjen]

While attempting to make all the tests pass, I have noticed a strange scenario where this 1 test was passing at random times when the tests are run despite me not changing the test code. The number of passing & failing tests were inconsistent each time. Reading the error message didn't help either because the tests weren't passing/failing consistently. After discussing with Bill, it turned out there was lag between the time the tests are run and data being fetched & loaded from the API; when the tests were run, it didn't need to go through any server but instead communicated directly with the API to fetch the data. It was this lag that was causing the inconsistency in the passing/failing of the tests. This is an issue called race condition. We had to re-structure app.js & async.test.js, server.jsand create a new Promise function to solve race condition bug. The following were the modifications made:

• new Deezer function to return a promise to load data (deezer.js)
• Modified tests to call this function before running any tests
• Modified server.js to call this before starting the server
• Stored melody as a new property of app e.g. app.set("musicChart", data)

I've also noticed one of the original test logic wasn't passing: test('should return an artist', async () => { const response = await request(app).get('/music'); console.log(response.body); expect(response.body.tracks.data[0].artist.name).toBe(true)

As I thought I have followed the correct property hierarchy. Bill then pointed out that there isn't an artist who is called true, meaning my logic was incorrect. If I were to stick with my original logic, I would need to check the length of the artist name; .length > 0 or .length !==0 But a more efficient way was to check if if the artist.name property existed;

test('should return an artist', async () => {
    const response = await request(app).get('/music');
    console.log(response.body);
    expect(response.body.tracks.data[0]).toHaveProperty('artist.name');
  });

This concludes the mini project. Switching over to travel-API.