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

Merge to Prod #93

Merged
merged 19 commits into from
Dec 22, 2020
Merged

Merge to Prod #93

merged 19 commits into from
Dec 22, 2020

Conversation

kwajiehao
Copy link
Contributor

@kwajiehao kwajiehao commented Dec 9, 2020

This release to production contains the following changes:

Bugs

Error-handling

Error-logging and debugging

Misc. improvements

Performance optimization

alexanderleegs and others added 15 commits November 24, 2020 15:24
* Feat: add new InputNameConflictError

* Fix: switch to error identification using HTTP error code
…e room (#80)

* Feat: create new Navigation class to load navigation file

* Feat: add to navigation on creation of new collection

* feat: add to navigation on creation of resource room

* Fix: remove redundant permalink field in config file

* Fix: use existing File to retrieve navigation instead

* Fix: use existing File to retrieve nav file for resourceroom

* Fix: remove unused class

* Fix: update nav file when deleting and renaming collectionPages

This commit also fixes a bug with the existing delete endpoint where the wrong key was being retrieved for the filename.

* Feat: handle nav changes on deleting and renaming resource room

* Fix: change for loop to filter/map instead
This commit logs the serialized error with a distinct string,
"Unrecognized internal server error:" so that we can create a cloudwatch
metric filter based on this string to identify and trigger alerts on
unrecognized / non-Isomer errors

Co-authored-by: Jie Hao Kwa <[email protected]>
* feat: add shareicon and analytics fields for Settings

* feat: retrieve and update agency logo in navigation.yml

This commit adds additional steps for the Settings route to retrieve
and update the navigation.yml file to update agency logo

* refactor: Settings class file retrieval and update process

This commit refactors the Settings class so that we can reuse a
utility function to retrieve the necessary settings files.

It also simplifies and standardizes the update process for all settings
files involved by putting the settings objects (config, navigation, footer)
in an array and looping over them.

* feat: update homepage title together with footer title

This commit resolves issue #269. If a user edits the title field,
they will update both the footer title and the browser title so that
the titles are consistent.

This commit achieves this by updating the title attribute of the homepage
file ONLY IF a change in the `title` field is detected.

* feat: send is_government field in response

This commit sends an additional is_government field when retrieving
settings configurations. The is_government field determines whether the
government masthead is displayed on the website.

* fix: error when updating settings object

* fix: retrieve settings configurations programmatically

Currently, we retrive the different settings configuration from
various files using an array. This is confusing since the settings files
are associated with random array indices. This commit switches to use
an object so that the operations to retrieve the files are associated
with a key instead.

* chore: replace arrays and magic indice numbers with objects

Previously, we used magic numbers to reference specific index
positions in arrays to retrieve data. This commit replaces such arrays
with objects so that data is accessible by key instead.

Co-authored-by: Jie Hao Kwa <[email protected]>
This commit fixes the behaviour for throwing NotFoundErrors in File. Previously, it was not possible to reach the condition to throw the NotFoundError in read, as a file which could not be retrieved would have have a defined endpoint. This resulted in a different error being thrown. This commit fixes that and also adds in the check during deleting and updating files.
* Feat: add endpoint to check if user has access to site

* Fix: remove log statement
* Feat: add util functions for git tree

* Feat: change collection rename endpoint to use git trees instead

* Fix: convert resource and resource room to use git trees instead

* Fix: remove unused variable

* Fix: move message into variable for readability
* feat: speed site retrieval up by making concurrent api calls

Previously, site retrieval was slow because the GitHub API for retrieving
org repos was paginated, and we retrieved the data sequentially,
one page at a time. This meant that it often took up to 7 or even 8 seconds
each time this endpoint is accessed (each page took around 3 seconds,
perhaps due to the large amount of data being sent).

This commit improves performance by making these api calls
concurrently, so that it now only takes around 3 seconds for the endpoint
to respond.

This commit also introduces an optional env var, ISOMERPAGES_REPO_PAGE_COUNT,
which determines how many pages of the GitHub API to comb simultaneously.
Since we know the number of repos our github org has, we can use this
info to speed up our endpoint by making concurrent calls instead of stepping
through the API pagination.

* refactor: remove unnecessary filter

Co-authored-by: Jie Hao Kwa <[email protected]>
The Settings class deals with three different types of configuration files:
_config.yml, _data/footer.yml, and _data/navigation.yml. Originally, we would
write to all three files, even if the only fields that were changed involved
only one of the files.

For example, even if I had only changed the `title` field (which affects only
the _config.yml file), I would end up making API calls to update the remaining
two settings files with their original content.

This commit allows us to save on wasted resources, and also increase the speed
of response by only updating a file if it needs to be updated.

Co-authored-by: Jie Hao Kwa <[email protected]>
* Feat: add separate handling for payloadTooLarge error in errorHandler

* Fix: swap intialization of cors
* Feat: split getRootTree function and add revertCommit function

This commit accomplishes 2 things: it takes the retrieval of commit and tree shas out of getRootTree (which obsoletes the function, since now getTree can be used), and adds a util function allowing a simple rollback of commits.

* Fix: update classes to use new implementation of util functions

* Feat: add route handler which allows for rollback

* Feat: update endpoints which may require rollback to use new handler

* Docs: update docs for sites endpoint

* Nit: fix revertCommit comment

* Fix: update rename pages to use rollback handler

* Fix: throw error instead of logging when encountered in getCommitAndTreeSha and getTree

* Chore: install new package

* Feat: add retry with exponential backoff for commit revertion

* Fix: rename error to be distinguish which error is being forwarded
This commit adds a `/v1` prefix to all URI paths to version our
API. This will be useful for several reasons:
- allows us to gracefully accommodate a v2 api which uses github's graphql API
- allows us to craft tight firewall rules (currently, we search for whether a
URI path contains `/sites` or `/auth`, which would allow nonsense URI paths like
`/authors`)

Co-authored-by: Jie Hao Kwa <[email protected]>
kwajiehao and others added 2 commits December 17, 2020 17:51
* feat: configure higher limit for nginx client_max_body_size

Even though we set a higher file size limit in our express app, we
found that uploads exceeding 1mb were being rejected by our server.
This was only occurring on the staging website, but not on local dev
env, which implies a server config issue.

After reading the nginx error logs, we found that nginx was rejecting
the requests because the body size of the request was too large. As a
result, we are introducing an ebextension which increases the max
body size (client_max_body_size) to 10mb (a slight increase over the 7mb
limit we set for express, since we want to be conservative and allow
for additional overhead).

Finally, the ebextension file includes a command to reload the nginx
service so that the new configs are used.

* chore: update gh workflow file to zip .ebextensions

Co-authored-by: Jie Hao Kwa <[email protected]>
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants