Refactor/netlify toml #357
Conversation
alexanderleegs
force-pushed
the
refactor/netlify-toml
branch
from
93ba5d0 to
d731692
Compare
newroutes/netlifyToml.js
Outdated
| const router = express.Router() | ||
|
|
||
| router.get( | ||
| "/netlify-toml", |
There was a problem hiding this comment.
we should really maintain a mapping of keys to raw values
There was a problem hiding this comment.
Could you clarify which raw values you're referring to?
There was a problem hiding this comment.
the /netlify-toml. idt it's a big issue cos the path should be used minimally? (lmk if i'm wrong)
There was a problem hiding this comment.
Right, this is only used for this single endpoint - do you think router namespaces will help with this issue?
| const netlifyTomlReadableContent = toml.parse(Base64.decode(content)) | ||
|
|
||
| // Headers is an array of objects, specifying a set of access rules for each specified path | ||
| // Under our current assumption, we apply the first set of access rules to all paths |
There was a problem hiding this comment.
i think i might be lacking context - could i clarify what's the current assumption here?
There was a problem hiding this comment.
Our assumption is that the file only contains a single set of access rules, so we take the first set!
There was a problem hiding this comment.
could we write that down in the comment too?
There was a problem hiding this comment.
Sure, done in 6517e43
alexanderleegs
force-pushed
the
refactor/netlify-toml
branch
from
a6cdec7 to
5517d45
Compare
newroutes/netlifyToml.js
Outdated
| const router = express.Router() | ||
|
|
||
| router.get( | ||
| "/netlify-toml", |
There was a problem hiding this comment.
the /netlify-toml. idt it's a big issue cos the path should be used minimally? (lmk if i'm wrong)
| const netlifyTomlReadableContent = toml.parse(Base64.decode(content)) | ||
|
|
||
| // Headers is an array of objects, specifying a set of access rules for each specified path | ||
| // Under our current assumption, we apply the first set of access rules to all paths |
There was a problem hiding this comment.
could we write that down in the comment too?
alexanderleegs
force-pushed
the
refactor/netlify-toml
branch
from
6517e43 to
a344e30
Compare
* Feat: add NetlifyTomlService * Feat: add netlifyToml router * Test: add new fixture for netlifyToml * Test: add tests for netlifyToml service and router * Fix: remove sitename from netlify-toml endpoint * Nit: add line space * Chore: include spec in name of test file and minor comment changes * Rebase: use auth verify middleware in router * Nit: add more details to comment * Fix: move routers to appropriate subrouters and swap to res.locals * Fix: axios mock in test for netlifytoml service
Overview
This PR introduces the refactor for the netlify toml retrieval flow. It introduces a newly created NetlifyTomlRouter and NetlifyTomlService.
Breaking Changes
Features:
Sites router
Notes:
The NetlifyToml router no longer uses the site name of the repo (compared to the v1 endpoint), as we retrieve this from the isomer-build repo rather than the individual site, and we don't actually use the siteName anywhere.