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

First steps in refactoring the backend code #213

Merged
merged 11 commits into from Nov 20, 2019

Conversation

@davidmehren
Copy link
Contributor

davidmehren commented Oct 27, 2019

This is the first of (hopefully) a few more PRs to refactor the backend code.

I am trying to work toward the following structure:

  • Routers only call methods on controllers
  • Controllers extract the required data from the HTTP request and pass them on. No req or res objects exist "beyond" the controllers.
  • The models only contain "simple" functionality, like "create", "find in database" etc. More complex functions should reside in separate helpers.

This isn't done yet (res and req objects are used pretty much everywhere), I plan to continue the work in the next weeks.

This PR contains these major changes:

  • The errors were extracted into a separate file.
  • All routes referenced from noteRouter were moved from response.js into files under lib/web/note
  • controller.js and slide.js are intended as controllers in the way outlined above. The naming isn't perfect yet and I will probably move slide-related stuff to it's own folder in the future.
davidmehren added 10 commits Oct 27, 2019
Because of circular import problems, this commit also moves the error messages from response.js to errors.js

Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
…scriptive

Move postNote to NoteController and rename to createFromPost

Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
Signed-off-by: David Mehren <dmehren1@gmail.com>
@davidmehren davidmehren requested a review from SISheogorath Oct 27, 2019
Signed-off-by: David Mehren <dmehren1@gmail.com>
@davidmehren davidmehren force-pushed the davidmehren:refactor_backend_notes branch from df4b8ed to b5cccef Oct 27, 2019
Copy link
Member

SISheogorath left a comment

In general this looks quite good. Seems to be mainly a renaming and moving of existing code.

Just a thought, we might want to use lib/web/note/index.js instead of lib/web/note/router.js. I'm not entirely sure about that idea, but at least want to mention it. How much do we want to make this directory a library in a long term perspective? Maybe @ccoenen has an opinion on that as well :)

All in all: 👍 But thumbs up and thanks a lot for your work!

if (fs.existsSync(path.join(__dirname, '..', 'public', 'build', 'reveal.js', 'css', 'theme', theme + '.css'))) {
return theme
}
return undefined

This comment has been minimized.

Copy link
@SISheogorath

SISheogorath Oct 27, 2019

Member

Just a thought, but writing undefined as theme, might be a bad idea? Does it fallback to the default then?

Even when this is most likely out of scope of this PR.

@SISheogorath SISheogorath merged commit 689f5a0 into codimd:master Nov 20, 2019
4 checks passed
4 checks passed
DCO DCO
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (codimd) No manifest changes detected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.