rithmschool / numbers_api Goto Github PK

since we're hosting on DigitalOcean, not sure whether we need this folder. Investigate.

Estimated points: 1

Problem description: Right now the API docs say that if you have a suggestion for adding a number fact, you should email it. But this isn't maintainable. It would be better if we could link people to this repo and encourage them to open up a PR with their new fact.

Problem description:

This file consists of js code that is meant to be shared across both front and back-end: https://github.com/rithmschool/numbers_api/blob/master/public/js/shared_utils.js

However, because it is served on the front-end, it has to be cross-browser compatible, may be more difficult to refactor, etc. Would be better to have this be in a node environment so that it's easier to upgrade.

Suggested solution:

We could follow this pattern

https://stackoverflow.com/questions/56591385/load-same-file-in-front-end-and-back-end

I recommend doing this before adding react so that the project is all ready to slot in a frontend folder. Would also allow us to refactor the file itself to a more modern style.

cuz jquery is so 2012.

Problem description:

frontend is old.

Suggested solution:

Refactor to a more modern architecture.

(See #47 for a related issue)

Problem description:
Currently lots of numbers are uninteresting.

Steps to Reproduce:
Lookup trivia for 2113.

The current behavior:
Returns “2113 is an unremarkable number.”

The expected behavior:
Return “2113 is a sum of 5 positive 5th powers.” e.g.

Suggested solution:
Include sequences from OEIS. E.g. there a lots of sequences containing 2113:
http://oeis.org/search?q=2113&sort=&language=english&go=Search

Problem description:
The original docs reference document.write, and jsonp.
These features are outdated and likely need to be removed from the project.

Suggested solution:
Remove this functionality from the codebase and update the docs.

Let's also format the code and do a general refactor: https://github.com/rithmschool/numbers_api/blob/master/public/js/script.js

Problem description:

There are several places in the docs where words are wrapped in backticks, eg here:

The backticks should be detected and parsed as code elements, as they are here for the words in front of a darker background.

Suggested solution:

Investigate and fix all occurrences so that the docs render properly.

Let's also format the code and do a general refactor:
https://github.com/rithmschool/numbers_api/blob/master/public/js/shared_utils.js

Edit: estimated 2 points

There's a bunch of commented-out code here:

https://github.com/rithmschool/numbers_api/blob/master/app.js#L21-L39

and here:

https://github.com/rithmschool/numbers_api/blob/master/app.js#L48-L91

would be nice to add this back. Right now, there are a couple of problems:

1. The API version has changed, so the commented out code is busted.
2. Would be better to not have code in a setInterval -- instead, let's just get the num shares from within the route handler as needed.

there's a bunch of stuff in /public that shouldn't be in version control (eg. bootstrap). Let's relegate as much as possible to node_modules.

This website has random facts about sequences of integers: https://oeis.org/
looks like they have some json api endpoints too: https://stackoverflow.com/questions/5991756/programmatic-access-to-on-line-encyclopedia-of-integer-sequences

It could be cool to have an endpoint to get a random fact about a sequence of numbers, eg. /v2/sequences?nums=1,2,7,10 would get a random sequence containing the numbers 1, 2, 7, and 10, and return info about it to the user

Problem description:

Related to #46 but should probably happen first. The app uses a weird pattern to attach the routes to the express app. See https://github.com/rithmschool/numbers_api/blob/master/app.js#L124 and https://github.com/rithmschool/numbers_api/blob/master/routes/api.js#L99.

Suggested solution:

Refactor to use express.Router. This will also be beneficial so that we can silo all of these routes and create a separate router for a v2 of the API.

Gather data from wikidata and create queries for art museums and tourist attractions. Get the location label, coordinates, and visitors of museums and tourist attractions. Sanitize the data and create queries that returns name, visitors, and coordinates based on the keyword that is inputted in the query. Write tests for graphQL.

Currently, a lot of tests are being run on the main database. This would cause issues when new data is scraped, because the data wouldn't match anymore. Create a dummy data file so the data doesn't change and refactor the tests based on the dummy data file.

Steps to Reproduce:
Understand the current testing structure.
Learn about the manual mocks that have been recently implemented.
https://jestjs.io/docs/manual-mocks

Suggested solution:
Expand coverage where needed.

Problem description:
Missing documentation of helper functions monthDaytoDayofYear() and dateToDayOfYear() in shared_utils.js

Suggested solution:
Add docstrings.

Relevant code snippet:

Estimated points: 1

Currently no routes have tests. Let's make sure the app is well-tested.

Estimated points: 6

This project needs some standard open-source documentation. Including:

• Code of conduct
• Contribution guidelines
• changelog
• License (MIT https://opensource.org/licenses/MIT)
• pull request template
• issue template

for the last two, see https://help.github.com/en/github/building-a-strong-community/about-issue-and-pull-request-templates

Problem description:

When scrolling/mouse-wheeling over the element with id #counter (see left side of screenshot), the number increments/decrements by one until your mouse-wheel action stops.

Steps to Reproduce:

The following steps can reproduce the issue locally.

1. In public/js/script.js, add a temporary console.log() to the .mousewheel callback function.
2. In routes/api.js, add a temporary console.log() to the factResponse function.
3. Start the server, and scroll over the element as described earlier. Observe the front-end and back-end console as you mouse-wheel.

The current behavior:

The current behavior currently calls the API from the current number(let's say 42) through to the number you stop at(let's say 1042) and can be done in a matter of seconds. That is one thousand API calls.

The expected behavior:

The API doesn't need to be called for every number, especially if the user changes the number multiple times within a second. At minimum, using the above scenario, the API should call for 42 and 1042. A better UX would be to call just a few in between as you increment.

Suggested solution:

Implement some debouncing for this feature to limit the amount of calls to one API call every ~0.3 - 1 seconds, or whatever amount feels right.
https://css-tricks.com/debouncing-throttling-explained-examples/

Relevant code snippet:

Seems like I can only use Http, looking to use Https

Problem description:

Readme does not describe requirements to get project running locally.

The current behavior:

Projects needs to run on older versions of node. Npm install results in many error messages and server does not start.

The expected behavior:

Unless otherwise noted, project should run on current stable versions of node.

Suggested solution:

Create section in ReadMe to describe setting up local environment.

Estimated points:

.5 point

Problem description:
The logic in these routes need to be separated so that they are easier to test.

Suggested solution:
Write tests as part of the refactor.

Problem description:
There are existing interactive requests in the docs but easy to miss.

Suggested solution:
Document the existing endpoints and create a sandbox to try the requests.
Something similar to the sandbox in the pokemon api https://pokeapi.co/

The expected behavior:
Refactor secrets.txt file to use a .env file with the dotenv library instead.
https://www.npmjs.com/package/dotenv

there's a function that used to be part of the project which would log every request (

However, the logging was consuming >100% CPU, mostly likely because the files were getting quite large and so reading / writing was expensive. Would be nice to bring this feature back, but we should think about how to better support it at current scale. (Redis? DB? something else?)

GraphQL needs testing on querying.

https://github.com/rithmschool/numbers_api/blob/master/models/data.js

Problem description:
When compiling the CSS, the .sass-cache directory is created, and is tracked by git.

Steps to Reproduce:

1. In the home directory of the repo, run compass compile public to generate the front end styling.
2. run git status and you will see the untracked .scssc files.

The current behavior:
The .scssc files are being tracked by git when we generate the files.

The expected behavior:
The .scssc files shouldn't be tracked by git when we generate the files.

Suggested solution:

add .sass-cache/ to the .gitignore file.

Relevant code snippet:

.sass-cache/

Estimated points:
0.01, since this is a one line fix.

Most of this code was written prior to ES2015, so it could use a refactor for 2020.

Problem description:

I don't know what's being used and what's not, but there's a smattering of random files in the project that could potentially be removed. eg this and this.

Suggested solution:

Analyze which files aren't being used, and DESROY THEMMMM

currently no tests to see if the dates are being returned correctly.
Ex: January 1st, February 29th

Problem description:

This file needs a fresh coat of paint: https://github.com/rithmschool/numbers_api/blob/master/models/fact.js

Suggested solution:

ES2015 refactor, no var keywords, address the todos, etc.

Problem description:
Right now the frontend uses jQuery and sass, there are no frontend tests.

Suggested solution:
Either test the existing jQuery code, or refactor the front end to use React and React testing library.
Any refactor should maintain the existing SASS code.

Sanitizing Data

Raw skyscraper data has been scraped from WikiData inside the wikidata folder under rawSkyscraper.js. The data needs to be sanitized.

The query used was this one here. At the current time of writing this, the height attribute is very inconsistent

For example:

The larger number (269) is the height in feet while the smaller number (82) is the height in meters.

However, not all entries with two heights go by this convention. The larger number (450) is the height at the top of the building in feet while the smaller number's (316) unit is still meters, but the height of the top floor.

One last example of inconsistent data points. The smaller number (442) is the height of the building in meters, but 1098 doesn't seem to be associated with this building, according to the Wikipedia article

Create function to parse data

Once the data is sanitized, create a function to parse the data out for endpoint usage. Maybe the structure of the return could look something like

[
  {
    name: "Zifeng Tower",
    height: int,
    elevators: int,
    coordinates: string
  },
  ...
]

Create GraphQL endpoint

Create a new type, query, and resolver for the GraphQL endpoint to point to the skyscraper data.

This API is a popular learning resource, so it would be cool if it could provide support for GraphQL as well! I don't really know what the types would look like, but for anyone who wants to learn about graphql, this could be a great opportunity.

some resources:

http://curric.rithmschool.com/meetups/lectures/graphql-intro/
https://developer.okta.com/blog/2018/09/27/build-a-simple-api-service-with-express-and-graphql

Problem description:

Related to #14 but different. The Python files in the models folder are Python2, old and in need of an update. Feels a bit strange to have Python files in here, but I'd be ok with an upgrade to Python3 or a re-implementation in node instead.

Suggested solution:

If we stay in python, we'll need to:

• upgrade to v3
• add requirements
• remove code by using packages (eg https://pypi.org/project/num2words/ and beautifulsoup)

if we move to node, we'll need to

• add cheerio as a dependency
• migrate each file to JS

We should break this up into sub-issues, but not until we make a decision about JS vs Python.

We don't currently have an automated deployment workflow for our hosting provider (Digital Ocean). Let's set up CI for this project.

Seems like types could help understand the code, based off comments like this: https://github.com/rithmschool/numbers_api/blob/master/models/data.js#L1-L19

Plenty of resources out there on Express+TS, here's one example: this could be useful: https://developer.okta.com/blog/2018/11/15/node-express-typescript

(Could also build the React frontend in TS as part of #53)

Current way graphQL is written is more akin to vanilla javascript. Makes it more difficult to find up-to-date documentation. Apollo has more extensive docs and more resources on writing tests for graphQL. Apollo will also be easier to implement with React as it has a Apollo React Client.

serverside: https://www.apollographql.com/docs/apollo-server/
react: https://www.apollographql.com/docs/react/

Problem description:
Need to figure out how to expand / improve facts data

The current behavior:
Facts are generated and stored in files on the server #14

The expected behavior:
Create a npm script gets new facts or updates current facts

502 Bad Gateway
nginx/1.4.6 (Ubuntu)

In express v2, you could use a function req.param() to access data coming from either the url or the query string. This was deprecated and now you just access req.params or req.query directly. however, in the express version bump i made for the project i just automatically changed req.param() to req.params, even if the data was coming from the query string. There may be bugs because of this, let's audit every place we reference req.params and be sure that we're trying to use a req parameter and not something else (e.g. a query string parameter)

There's a bunch of python files in /models used for scraping -- but it seems like these were just run in order to generate text files. Let's clarify how these files fit into the general structure of the project. Would be good to add to the Wiki on GitHub explaining this.

There's currently a Gemfile in the project for static asset compilation (I think SCSS to CSS)? Whatever the case, let's get rid of this -- if we need SCSS compilation, let's do it strictly from within the node ecosystem.

The docs state that you can set the response to be json by setting a content-type header in the request. However, content-type is typically for the content type of the request data, not the response data. For the latter, we should use the Accept header. We should modify the docs and the server to support setting the Accept header (right now i don't think you can set either header via ajax bc cors).

Longer term we should also remove this feature when setting content-type and just use accept. But since this introduces a breaking change, we should reserve it for a major version change (i.e. not v1.1)

Problem description:

I see one example of a function with jsdoc-style documentation: https://github.com/rithmschool/numbers_api/blob/master/models/fact.js#L145-L157

since this is an open source project, we really should do our best to provide thorough and useful documentation of all of the functions in the project.

Suggested solution:

write documentation consistent with the example linked above!

The README needs to have line breaks between all section headings, otherwise the markdown won't get parsed correctly. Here's an example (we shouldn't see backticks around json, for example):

Anything we can do in here, let's do from the scripts key in package.json

You can get facts by number, but it could be nice to implement a search feature where you can search for a keyword (e.g. baseball) and get a list of facts that match that keyword.

This might be more work than it seems if searching through the text files is slow. But we can save optimization for a future issue.