actualbudget / actual-server

Actual's server
https://actualbudget.org
MIT License
3.09k stars 588 forks source link

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

Open tcrasset opened 4 weeks ago

tcrasset commented 4 weeks ago

Verified issue does not already exist?

What happened?

I have read through the code to see how one could export an OpenApi schema as asked by https://github.com/actualbudget/actual-server/issues/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:

https://github.com/actualbudget/actual-server/blob/eec5fbb1cc515cc4a2495cc2c683024e5446fb88/src/app-sync.js#L24

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 commented 4 weeks ago

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