Added git services, db caching and front end API changes #57

asiyani · 2017-06-11T17:22:29Z

After merging this pull request Frontend will get data from backend DB.
Created Search API for front-end - (This one is not GraphQL) but some this for now just to use same front end without much modification.

Folders added.

gitUtility in server folder
- gitService file will make external git API calls. 2 function we can use getUserDetails(login, token) & getStarredRepository(login, token, page = 1).
- initialSeed.js for initail seeding data or udpating records in future.

created static method updateRepoWithStargazer in repositorySchema which we can use to update records in DB. this method will link Stargazer to Repos being updated.

Added repo APIs.
- GET /api/repos/v1/search to search db with query parameter stargazer, language, page, per_page. this is to replicate current API we are doing to Github.
- GET /api/repos/v1/updateRecord this is to start DB seeding.

Note:

We need to seed our DB for /api/repos/v1/search to work. To do that i have created simple call we can make from the browser (after login in).
Just go to http://localhost:3000/api/repos/v1/updateRecord. this will start seeding process and you will get logs on the terminal.
I am doing seeding for ALL starred records. some username got about 1K+ starred repos.I got about 11K repos documents in Repository collections. This process might take some time.

After seeding data front end will work as it is working now- just data will come from our db.
Since we are getting data from our server we can do following.

In the front-end, we need to change they way we do API calls when language is selected. Since now we can pass language as search parameter No need to filter that on the front-end.

Please let me know if any issue or what you guys think about this process of getting data.

mubaris · 2017-06-12T05:45:45Z

It didn't work until I called, /api/repos/v1/updateRecord manually. We need to find a method to overcome this.

Language selection is not working.

After a point application exits due to undefined response body

mubaris

It didn't work until I called, /api/repos/v1/updateRecord manually. We need to find a method to overcome this.

Language selection is not working.

After a point application exits due to undefined response body

asiyani · 2017-06-12T06:15:00Z

Ya some of the language doesn't work becz dB doesn't have any repo for that language.
Which one you tried?does it work on previous front end?

mubaris · 2017-06-12T08:55:19Z

It's not working for Javascript. It was working previously.

asiyani · 2017-06-12T09:16:00Z

I think its something to do with scroll, if you scroll down it starts doing API calls. Not sure why this is happening....

mubaris · 2017-06-12T09:17:00Z

It's working now. I don't know why it was happening.

mubaris · 2017-06-12T09:17:54Z

Username array isn't stored in DB. Is it?

asiyani · 2017-06-12T09:26:51Z

No its just a file for now, but DB got all github info regarding those usernames.....

mubaris · 2017-06-12T09:41:50Z

If the user wants to add new username, it's not possible now.

asiyani · 2017-06-12T09:57:47Z

Regarding adding username on end user request is not yet possible but I was Thinking if user wants to add username we will let him add that but only for him like temp data.
we will add it to our DB only if they it meet certain criteria. something like how many follower he got or count of repo. Otherwise our starzagers list will grow and we will loose quality.

alejandronanez · 2017-06-15T10:15:33Z

server/routes/api/repos.js

+ }
+ console.log('isAuthenticated: No');
+ res.redirect('/user/login');
+ return 0;


Any reason to return 0 here?

This function will never reach return statement. either it will be redirected if not authenticated or next() will be called

Gotcha, so I think there's no reason to call a return in here then (dead code)

alejandronanez · 2017-06-15T10:16:10Z

server/routes/api/repos.js

+ query.stargazersLogin = { $all: [req.query.stargazer] };
+ }
+ // Filter language depending on what selected.
+ if (req.query.language && req.query.language !== 'Any') {


Any && NoLanguage can live in their own file, something like constants/index.js

alejandronanez · 2017-06-15T10:16:55Z

server/routes/api/repos.js

+ }
+ }
+ Repository.paginate(query,
+ { page: parseInt(req.query.page, 10) || 1,


Do you mind changing this into:

{ page: ..., limit: .... }

I am sorry I didn't understand what you mean here...

Beautify the code 😆

alejandronanez · 2017-06-15T10:18:44Z

server/gitUtility/initialSeed.js

+ { new: true, upsert: true },
+ (error, doc) => {
+ if (error) throw error;
+ //logger.debug(`Updated: ${stargazer}`);


Can you remove this commented code please? Unless it's really necessary for the future.

alejandronanez · 2017-06-15T10:19:22Z

config/winston.js

+
+logger.level = 'info';
+
+// logger.debug('Test Log Message', { anything: 'This is metadata' });


Commented out code.

alejandronanez · 2017-06-15T10:19:57Z

public/js/languageSelection.js

@@ -42,7 +42,7 @@ const selectLanguage = function selectLanguage(event) {
 languageSelected = event.target.value;
 content.innerHTML = '';
 reqNo = Math.floor(Math.random() * 3) + 1;
- projectsPerPage = (languageSelected == ANY_LANGUAGE) ? 2 : 100;
+ projectsPerPage = (languageSelected == ANY_LANGUAGE) ? 2 : 10;


You can mode 10 into const MAX_PROJECTS = 10 or something similar.

alejandronanez · 2017-06-15T10:20:50Z

public/js/main.js

 innerContent += (entry.language != null) ? `&emsp;${entry.language}` : '';
- innerContent += `&emsp;(from ${userFormatter(username)})`;
+ // innerContent += `&emsp;(from ${userFormatter(username)})`;


Remove this commented out code here.

alejandronanez · 2017-06-15T10:21:35Z

public/js/main.js

@@ -63,8 +68,12 @@ const getData = function getData() {
 usersCurrentCall = 0;
 callInProgress = true;
 reqNo += 1;
+ if (reqNo > 10) {


maybe use something more descriptive as const REQ_NUMBER???

alejandronanez · 2017-06-15T10:22:00Z

public/js/main.js

 USERNAMES.forEach((username) => {
- const url = `https://api.github.com/users/${username}/starred?per_page=${projectsPerPage}&access_token=${accessToken}&page=${reqNo}`;
+ // const url = `https://api.github.com/users/${username}/starred?per_page=${projectsPerPage}&access_token=${accessToken}&page=${reqNo}`;


This is commented code that I think it's not necessary, can you remove it?

alejandronanez · 2017-06-15T10:22:19Z

server/db/mongoose.js

@@ -1,4 +1,4 @@
-let mongoose = require('mongoose');
+const mongoose = require('mongoose');


alejandronanez · 2017-06-15T10:23:11Z

server/db/repositories.js

+ if (repository.stargazersLogin.indexOf(stargazer) === -1) {
+ repository.stargazersLogin.push(stargazer);
+ }
+ Object.assign(repository, repo);


You should better do Object.assign({}, repository, repo) to avoid any undesired mutation.

alejandronanez · 2017-06-15T10:24:23Z

server/db/stargazers.js

+ public_repos: Number,
+ public_gists: Number,
+ followers: Number,
+}, { timestamps: { updatedAt: 'recordUpdated_at', createdAt: 'recordCreated_at' } });


I think you can write this object in multiple lines. A one liner is harder to read + the diff in the future might not be clear.

alejandronanez · 2017-06-15T10:25:23Z

server/gitUtility/gitService.js

+
+ getUserDetails(login, token) {
+ if (token) {
+ headers = {


I don't see where headers is defined? Am I missing something here?

I have added header definition now...

alejandronanez · 2017-06-15T10:26:18Z

server/gitUtility/gitService.js

+ return new Promise((resolve, reject) => {
+ logger.info('Calling api.github.com/users/user', { login });
+ request(options, (error, response, body) => {
+ if (!error && response.statusCode == 200) {


Always use === instead of ==, using == can led to bugs most of times.

alejandronanez · 2017-06-15T10:26:53Z

server/gitUtility/gitService.js

+ return new Promise((resolve, reject) => {
+ logger.info('Calling api.github.com/users/login/starred', { login, page });
+ request(options, (error, response, body) => {
+ if (!error && response.statusCode == 200) {


Same comment for ===

alejandronanez · 2017-06-15T10:30:56Z

.eslintrc.json

@@ -16,6 +16,7 @@
 "no-console": "off",
 "func-names": "off",
 "no-alert": "off",
- "prefer-const": "off"
+ "prefer-const": ["error", {"destructuring": "all"}],


I'm not really sure about this change, any specific reason for this update?

I have added that to warn us if let is used and we never resigned that variable.... {"destructuring": "all"} because of the following....

/*eslint prefer-const: ["error", {"destructuring": "all"}]*/ /*eslint-env es6*/ // 'b' is never reassigned, but all of `a` and `b` should not be const, so those are ignored. let {a, b} = obj; a = a + 1;

I think we should avoid this, if we mutate our data some weird bugs may arise.

mubaris · 2017-06-20T02:46:21Z

@alejandronanez @asiyani Status?

alejandronanez

Enforce const { foo } = bar. Mutate our data is not good.

alejandronanez · 2017-06-20T03:39:01Z

.eslintrc.json

@@ -16,6 +16,7 @@
 "no-console": "off",
 "func-names": "off",
 "no-alert": "off",
- "prefer-const": "off"
+ "prefer-const": ["error", {"destructuring": "all"}],


I think we should avoid this, if we mutate our data some weird bugs may arise.

asiyani · 2017-06-20T21:03:28Z

Sorry guys have been busy for last few days. I have made all changes now.
Let me know if anything else needs changing.

asiyani · 2017-06-30T09:30:04Z

Hi guys did you had time to review this?
Let me know if any issue

asiyani added 9 commits June 10, 2017 11:58

changed username shchema and route to Stargazers

bc19f87

modified stargazers and repository schema relation

39227b2

added timeStamp and static function for repository model

d32db3c

added git utility - service to get data from github

44af092

removed token and replaced with null

b0cd898

created serach API for repo

2ad21ac

added git service for git API and created end point for update.

1e3ae42

changed front end to use backend search API

6cb90d6

added limit of 10 for search request

3a49382

mubaris requested review from mubaris and alejandronanez June 12, 2017 05:30

mubaris reviewed Jun 12, 2017

View reviewed changes

mubaris approved these changes Jun 12, 2017

View reviewed changes

alejandronanez reviewed Jun 15, 2017

View reviewed changes

server/db/mongoose.js

@@ -1,4 +1,4 @@

let mongoose = require('mongoose');

const mongoose = require('mongoose');

Copy link

Contributor

alejandronanez Jun 15, 2017

Choose a reason for hiding this comment

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

💯

alejandronanez reviewed Jun 15, 2017

View reviewed changes

alejandronanez suggested changes Jun 15, 2017

View reviewed changes

code review changes

e4d76c2

alejandronanez suggested changes Jun 20, 2017

View reviewed changes

asiyani added 2 commits June 20, 2017 21:58

review changes and added constants.js

94eec53

removed return from isAuthenticated

16b21d3


		logger.level = 'info';

		// logger.debug('Test Log Message', { anything: 'This is metadata' });

		@@ -1,4 +1,4 @@
		let mongoose = require('mongoose');
		const mongoose = require('mongoose');

Added git services, db caching and front end API changes #57

Are you sure you want to change the base?

Added git services, db caching and front end API changes #57

Conversation

asiyani commented Jun 11, 2017 • edited

Folders added.

Note:

mubaris commented Jun 12, 2017

mubaris left a comment

Choose a reason for hiding this comment

asiyani commented Jun 12, 2017

mubaris commented Jun 12, 2017

asiyani commented Jun 12, 2017

mubaris commented Jun 12, 2017

mubaris commented Jun 12, 2017

asiyani commented Jun 12, 2017

mubaris commented Jun 12, 2017

asiyani commented Jun 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mubaris commented Jun 20, 2017

alejandronanez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asiyani commented Jun 20, 2017

asiyani commented Jun 30, 2017

asiyani commented Jun 11, 2017 •

edited