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

[Maintenance]: apply separation of concerns to this repo #420

Open
1 task done
tcrasset opened this issue Aug 14, 2024 · 1 comment
Open
1 task done

[Maintenance]: apply separation of concerns to this repo #420

tcrasset opened this issue Aug 14, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@tcrasset
Copy link
Contributor

Verified issue does not already exist?

  • I have searched and found no existing issue

What happened?

I have read through the code to see how one could export an OpenApi schema as asked by #419, but what I've found is that there is a deeper issue going on in this repo.

That is, separating concerns in the app, having code whose purpose is handling the incoming HTTP route, code whose purpose it generating a response, and code whose purpose is accessing the database.

Currently, most of the endpoints have all this in a single function. Example:

app.post('/sync', async (req, res) => {

There seem to be some files that are going in a positive direction, e.g.

https://github.com/actualbudget/actual-server/blob/master/src/services/secrets-service.js

but it does not seem to be a majority, as far as I could tell.

I'm not criticizing the work that has already been done, obviously there is a lot of work behind all of it, and it works!

However, going forward to be able to scale we would need to have some separation to make addition of routes, testing, adding an OpenAPI spec, much easier.

I would be glad to give a hand, I have dabbled a bit in JS, but my day job consist of being a backend developer in Python, so I know my way around a clean architecture, I'll just need to apply it to JS, similar to the secrets-service.js shown above.

What error did you receive?

No response

Where are you hosting Actual?

None

What browsers are you seeing the problem on?

No response

Operating System

None

@tcrasset tcrasset added the bug Something isn't working label Aug 14, 2024
@tcrasset
Copy link
Contributor Author

Example of a blog post of how to apply this to an Express JS app:

https://medium.com/@ben.dev.io/clean-architecture-in-node-js-39c3358d46f3

@tcrasset tcrasset changed the title [Feature Request]: apply separation of concerns to this repo [Maintenance]: apply separation of concerns to this repo Aug 15, 2024
This was referenced Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant