Move sync-server into Actual repo #4334

MikesGlitch · 2025-02-07T19:14:07Z

Bringing the Actual Sync Server into the Actual workspace.

Looking for 3+ approvers before merging.

I had to update

Docker - updating paths because sync-server now lives in the packages folder
Git workflows - removed workflows that were already in Actual repo and updating references in the remaining workflows
Server loadConfig.js - changed the way it finds the Actual web client - due to the way node_modules are different in workspaces
Added yarn start:server and yarn install:server for some easy of use. yarn install:server isn't required, but it only installs servers dependencies so it'll be a faster/smaller download.

GitHub CI log: ``` [linux/arm/v6 base 6/8] RUN yarn workspaces focus --all --production 204.6 ➤ YN0007: │ bcrypt@npm:5.1.0 must be built because it never has been before or the last one failed 204.6 ➤ YN0007: │ better-sqlite3@npm:8.2.0 must be built because it never has been before or the last one failed ... [linux/arm/v7 base 6/8] RUN yarn workspaces focus --all --production 203.8 ➤ YN0007: │ bcrypt@npm:5.1.0 must be built because it never has been before or the last one failed 203.8 ➤ YN0007: │ better-sqlite3@npm:8.2.0 must be built because it never has been before or the last one failed ``` It seems that both armv6 and armv7 have the same issues with `bcrypt` and `better-sqlite3` not being built. These packages are required to build from source, luckily QEMU use armv7l for compiling. Tested and working on RPi Zero W. --------- Co-authored-by: Jed Fox <[email protected]>

Seems like we already added `tsc` to build the project, but we use the wrong babel preset (Flow) instead of the specific TS one. This is only used in testing to make Jest work (from what I can tell).

CodeQL keeps yelling at us about this… I’m not sure if the filter is smart enough to use this rate limit middleware to remove the warnings, but at least we will be setting a reasonable bound on attempts to crack the server password.

Co-authored-by: Matiss Janis Aboltins <[email protected]>

…192) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…(#190) Helps with actualbudget#919 by adding the `all` field wit both pending and booked transactions to the output of `getTransactionsWithBalance()` and, by extension, the `/nordigen/transactions` endpoint. I could alter the `getTransactions()` to return the `all` field as well but I figured that keeping it such that it returns the output from Nordigen API 1:1 might be better so I left it as is. If you don't agree, let me know and I'll update this.

Co-authored-by: Henrik Maaland <[email protected]>

A small fix: returning JSON response instead of plain-text. The frontend always expects a JSON response. So a tiny fix here..

Previously, the latest artifact list was requested unauthenticated using `ADD "https://api.github.com/..." /tmp/artifacts.json`. While this works locally, on GitHub’s servers it seems that the per-IP rate limit was exceeded. There isn’t a way to get Docker to pass the `Authorization` header that I know of, so this work has been moved to an external shell script that pulls down the relevant data.

…#208)

Web: actualbudget#1087 Server: actualbudget/actual-server#207 Docs: actualbudget/docs#179 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Remove the bulk in favour of links to our core docs. --------- Co-authored-by: Jed Fox <[email protected]>

- actualbudget/docs#202

Replaced contributing link

This allows running a health check from inside the container. Usage: `npm run health-check`. That may not work inside of Alpine containers, so you can do `node src/scripts/health-check.js` directly instead. Fixes actualbudget#213.

Using the new CRDT package instead of API.

- web: actualbudget#1280 - server: actualbudget/actual-server#222 - docs: actualbudget/docs#223 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

- web: actualbudget#1295 - server: actualbudget/actual-server#224 - docs: actualbudget/docs#225

…budget#225) - web: actualbudget#1322 - server: actualbudget/actual-server#225 - docs: actualbudget/docs#230

1. upgrade `nordigen-node` to 1.2.6 (which uses the new gocardless domain) 2. allow accessing `nordigen` functionality via `/gocardless` to unblock using the new API path in actual-web

matt-fidd

Pulled down and it runs well! install:server is a great idea, makes it much smaller/quicker for people not interested in dev.

I tested SimpleFIN, GoCardless and OpenID and all worked as expected, alongside the usual Actual stuff.

MatissJanis

Most of my comments are nitpicks that can be addressed post-merge.

The only blocker I would say is: aligning the prettier config. It will be easier to align the prettier config in this PR (& re-run prettier auto-fixer) than to do this later and submit another mega-pr with all the code-style modifications.

MatissJanis · 2025-02-10T18:29:58Z

packages/loot-core/package.json

@@ -41,6 +41,7 @@
  "devDependencies": {
    "@actual-app/api": "workspace:^",
    "@actual-app/crdt": "workspace:^",
+    "@actual-app/web": "workspace:^",


This is not great 😬
(but I get that your changes in this PR don't actually introduce this)

MatissJanis · 2025-02-10T18:30:28Z

packages/sync-server/.dockerignore

I think we can move this file to root folder
(but I might be wrong)

MatissJanis · 2025-02-10T18:30:38Z

packages/sync-server/.editorconfig

MatissJanis · 2025-02-10T18:30:52Z

packages/sync-server/.eslintignore

MatissJanis · 2025-02-10T18:31:00Z

packages/sync-server/.eslintrc.cjs

MatissJanis · 2025-02-10T18:31:30Z

packages/sync-server/.prettierrc.json

MatissJanis · 2025-02-10T18:32:30Z

packages/sync-server/LICENSE.txt

I think we can delete this file. It's the same as the LICENSE in the root dir.

MatissJanis · 2025-02-10T18:33:57Z

packages/sync-server/package.json

@@ -0,0 +1,65 @@
+{
+  "name": "actual-sync",


💬 suggestion: to align with the other pkg names‏

Suggested change

"name": "actual-sync",

"name": "@actual-app/sync-server",

Will tackle this in a follow up just to be safe

MatissJanis · 2025-02-10T18:37:32Z

packages/sync-server/upcoming-release-notes/557.md

Are these release note files supposed to be here? Should we instead move them to the root folder?

I moved them to the root folder

* feedback * remove temp script

packages/sync-server/src/app-simplefin/app-simplefin.js

+    const req = https.request(new URL(token), options, res => {
+      res.on('data', d => {
+        resolve(d.toString());
+      });
+    });


packages/sync-server/src/app-simplefin/app-simplefin.js

+    const req = https.request(
+      new URL(`${sfin.baseUrl}/accounts${queryString}`),
+      options,
+      res => {
+        let data = '';
+        res.on('data', d => {
+          data += d;
+        });
+        res.on('end', () => {
+          if (res.statusCode === 403) {
+            reject(new Error('Forbidden'));
+          } else {
+            try {
+              const results = JSON.parse(data);
+              results.sferrors = results.errors;
+              results.hasError = false;
+              results.errors = {};
+              resolve(results);
+            } catch (e) {
+              console.log(`Error parsing JSON response: ${data}`);
+              reject(e);
+            }
+          }
+        });
+      },
+    );


intiplink and others added 30 commits April 7, 2023 14:37

Don’t check for release notes on release/* branches (#181)

6f7306c

Blank commit to trigger image build

1504127

Add workflow_dispatch to edge image action so it can be manually run

0096907

github action workflow: revert old way to build images (#184)

82b4643

Use babel preset-typescript (#185)

47d77a3

Seems like we already added `tsc` to build the project, but we use the wrong babel preset (Flow) instead of the specific TS one. This is only used in testing to make Jest work (from what I can tell).

Add express-rate-limit (#187)

a1af1ff

CodeQL keeps yelling at us about this… I’m not sure if the filter is smart enough to use this rate limit middleware to remove the warnings, but at least we will be setting a reasonable bound on attempts to crack the server password.

🔖 (23.4.1) fix rule creation from transaction list (actualbudget#188)

bc93604

Add a password set/reset script (#186)

104c980

Co-authored-by: Matiss Janis Aboltins <[email protected]>

Add a handler for AxiosError errors (actualbudget#189)

652d75a

🔖 (23.4.2) revert back to old autocomplete & keyboard shortcut fix (#…

2f54a94

…192) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Remove the unused migrations folder (actualbudget#195)

996f238

App secret service (actualbudget#194)

d60bb6a

Co-authored-by: Henrik Maaland <[email protected]>

🐛 (nordigen) return JSON response for the secrets API (#197)

fb6dc5e

A small fix: returning JSON response instead of plain-text. The frontend always expects a JSON response. So a tiny fix here..

🔖 (23.5.0) various improvements (actualbudget#200)

9169bfa

Update README (actualbudget#201)

5d36ecb

Migrate to combined release notes action from main repo (actualbudget…

d1ff068

…#208)

🔖 (23.6.0) category hiding and filters for reports (actualbudget#207)

7c8e9bb

Web: actualbudget#1087 Server: actualbudget/actual-server#207 Docs: actualbudget/docs#179 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Update README.md (#215)

8ac74ed

Remove the bulk in favour of links to our core docs. --------- Co-authored-by: Jed Fox <[email protected]>

Update “how it works” link (#216)

ca27949

- actualbudget/docs#202

Add a health check script (actualbudget#214)

5252f45

This allows running a health check from inside the container. Usage: `npm run health-check`. That may not work inside of Alpine containers, so you can do `node src/scripts/health-check.js` directly instead. Fixes actualbudget#213.

♻️ (crdt) use actual-app/crdt package instead of api (actualbudget#218)

debb33a

Using the new CRDT package instead of API.

Update comments in docker-compose.yml (actualbudget#221)

28e0d71

🔖 (23.7.0) Nordigen release, stability improvements (actualbudget#222)

8237eb5

- web: actualbudget#1280 - server: actualbudget/actual-server#222 - docs: actualbudget/docs#223 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

🔖 (23.7.1) critical sync fixes (actualbudget#224)

c2ee928

- web: actualbudget#1295 - server: actualbudget/actual-server#224 - docs: actualbudget/docs#225

🔖 (23.7.2) more patches (not so critical, but still annoying) (actual…

354152d

…budget#225) - web: actualbudget#1322 - server: actualbudget/actual-server#225 - docs: actualbudget/docs#230

⬆️ (nordigen-node) upgrade to v1.2.6 (actualbudget#229)

1de788e

1. upgrade `nordigen-node` to 1.2.6 (which uses the new gocardless domain) 2. allow accessing `nordigen` functionality via `/gocardless` to unblock using the new API path in actual-web

actual-github-bot bot added ✅ Approved and removed 🔍 Ready for review labels Feb 9, 2025

matt-fidd previously approved these changes Feb 9, 2025

View reviewed changes

actual-github-bot bot added 🔍 Ready for review and removed ✅ Approved labels Feb 9, 2025

MatissJanis requested changes Feb 10, 2025

View reviewed changes

actual-github-bot bot added ⚠️ Changes requested and removed 🔍 Ready for review labels Feb 10, 2025

feedback (#5)

7faecf4

* feedback * remove temp script

MikesGlitch dismissed stale reviews from matt-fidd and jfdoming via 7faecf4 February 10, 2025 19:26

actual-github-bot bot added 🔍 Ready for review and removed ⚠️ Changes requested labels Feb 10, 2025

github-advanced-security bot found potential problems Feb 10, 2025

View reviewed changes

MikesGlitch changed the title ~~Merge sync-server into actual~~ Move sync-server into Actual repo Feb 10, 2025

MatissJanis previously approved these changes Feb 10, 2025

View reviewed changes

actual-github-bot bot added ✅ Approved and removed 🔍 Ready for review labels Feb 10, 2025

bringing in the lastest updates

7ddd79c

MikesGlitch dismissed MatissJanis’s stale review via 7ddd79c February 10, 2025 20:11

actual-github-bot bot added 🔍 Ready for review and removed ✅ Approved labels Feb 10, 2025

MatissJanis approved these changes Feb 10, 2025

View reviewed changes

actual-github-bot bot added ✅ Approved and removed 🔍 Ready for review labels Feb 10, 2025

MikesGlitch merged commit ac08b87 into actualbudget:master Feb 10, 2025
22 of 23 checks passed

actual-github-bot bot added ✨ Merged and removed ✅ Approved labels Feb 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move sync-server into Actual repo #4334

Move sync-server into Actual repo #4334

MikesGlitch commented Feb 7, 2025 •

edited

Loading

matt-fidd left a comment

MatissJanis left a comment

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MatissJanis Feb 10, 2025

MikesGlitch Feb 10, 2025

MatissJanis Feb 10, 2025

MikesGlitch Feb 10, 2025

Move sync-server into Actual repo #4334

Move sync-server into Actual repo #4334

Conversation

MikesGlitch commented Feb 7, 2025 • edited Loading

matt-fidd left a comment

Choose a reason for hiding this comment

MatissJanis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikesGlitch commented Feb 7, 2025 •

edited

Loading