Skip to content

Commit

Permalink
Merge branch 'master' into optimise-gocardless-2
Browse files Browse the repository at this point in the history
  • Loading branch information
matt-fidd authored Aug 17, 2024
2 parents 290d0d7 + 8201085 commit 7362262
Show file tree
Hide file tree
Showing 8 changed files with 209 additions and 7 deletions.
12 changes: 11 additions & 1 deletion .github/workflows/stale.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,20 @@ jobs:
stale:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v8
- uses: actions/stale@v9
with:
stale-pr-message: 'This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.'
close-pr-message: 'This PR was closed because it has been stalled for 5 days with no activity.'
days-before-stale: 30
days-before-close: 5
days-before-issue-stale: -1
stale-wip:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v9
with:
stale-pr-message: ':wave: Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review.'
days-before-stale: 7
any-of-labels: ':construction: WIP'
days-before-close: -1
days-before-issue-stale: -1
9 changes: 7 additions & 2 deletions src/app-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { SyncProtoBuf } from '@actual-app/crdt';

const app = express();
app.use(errorMiddleware);
app.use(express.json());
app.use(express.raw({ type: 'application/actual-sync' }));
app.use(express.json());

app.use(validateUserMiddleware);
export { app as handlers };
Expand All @@ -31,6 +31,7 @@ app.post('/sync', async (req, res) => {
try {
requestPb = SyncProtoBuf.SyncRequest.deserializeBinary(req.body);
} catch (e) {
console.log('Error parsing sync request', e);
res.status(500);
res.send({ status: 'error', reason: 'internal-error' });
return;
Expand All @@ -44,7 +45,11 @@ app.post('/sync', async (req, res) => {
let messages = requestPb.getMessagesList();

if (!since) {
throw new Error('`since` is required');
return res.status(422).send({
details: 'since-required',
reason: 'unprocessable-entity',
status: 'error',
});
}

let currentFiles = accountDb.all(
Expand Down
168 changes: 168 additions & 0 deletions src/app-sync.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import request from 'supertest';
import { handlers as app } from './app-sync.js';
import getAccountDb from './account-db.js';
import { getPathForUserFile } from './util/paths.js';
import { SyncProtoBuf } from '@actual-app/crdt';
import crypto from 'node:crypto';

describe('/download-user-file', () => {
describe('default version', () => {
Expand Down Expand Up @@ -137,3 +139,169 @@ describe('/delete-user-file', () => {
expect(rows[0].deleted).toBe(1);
});
});

describe('/sync', () => {
it('returns 401 if the user is not authenticated', async () => {
const res = await request(app).post('/sync');

expect(res.statusCode).toEqual(401);
expect(res.body).toEqual({
details: 'token-not-found',
reason: 'unauthorized',
status: 'error',
});
});

it('returns 200 and syncs successfully with correct file attributes', async () => {
const fileId = crypto.randomBytes(16).toString('hex');
const groupId = 'group-id';
const keyId = 'key-id';
const syncVersion = 2;
const encryptMeta = JSON.stringify({ keyId });

addMockFile(fileId, groupId, keyId, encryptMeta, syncVersion);

const syncRequest = createMinimalSyncRequest(fileId, groupId, keyId);

const res = await sendSyncRequest(syncRequest);

expect(res.statusCode).toEqual(200);
expect(res.headers['content-type']).toEqual('application/actual-sync');
expect(res.headers['x-actual-sync-method']).toEqual('simple');
});

it('returns 500 if the request body is invalid', async () => {
const res = await request(app)
.post('/sync')
.set('x-actual-token', 'valid-token')
// Content-Type is set correctly, but the body cannot be deserialized
.set('Content-Type', 'application/actual-sync')
.send('invalid-body');

expect(res.statusCode).toEqual(500);
expect(res.body).toEqual({
status: 'error',
reason: 'internal-error',
});
});

it('returns 422 if since is not provided', async () => {
const syncRequest = createMinimalSyncRequest(
'file-id',
'group-id',
'key-id',
);
syncRequest.setSince(undefined);

const res = await sendSyncRequest(syncRequest);

expect(res.statusCode).toEqual(422);
expect(res.body).toEqual({
status: 'error',
reason: 'unprocessable-entity',
details: 'since-required',
});
});

it('returns 400 if the file does not exist in the database', async () => {
const syncRequest = createMinimalSyncRequest(
'non-existant-file-id',
'group-id',
'key-id',
);

// We do not insert the file into the database, so it does not exist

const res = await sendSyncRequest(syncRequest);

expect(res.statusCode).toEqual(400);
expect(res.text).toEqual('file-not-found');
});

it('returns 400 if the file sync version is old', async () => {
const fileId = crypto.randomBytes(16).toString('hex');
const groupId = 'group-id';
const keyId = 'key-id';
const oldSyncVersion = 1; // Assuming SYNC_FORMAT_VERSION is 2

// Add a mock file with an old sync version
addMockFile(
fileId,
groupId,
keyId,
JSON.stringify({ keyId }),
oldSyncVersion,
);

const syncRequest = createMinimalSyncRequest(fileId, groupId, keyId);

const res = await sendSyncRequest(syncRequest);

expect(res.statusCode).toEqual(400);
expect(res.text).toEqual('file-old-version');
});

it('returns 400 if the file needs to be uploaded (no group_id)', async () => {
const fileId = crypto.randomBytes(16).toString('hex');
const groupId = null; // No group ID
const keyId = 'key-id';
const syncVersion = 2;

addMockFile(fileId, groupId, keyId, JSON.stringify({ keyId }), syncVersion);

const syncRequest = createMinimalSyncRequest(fileId, groupId, keyId);

const res = await sendSyncRequest(syncRequest);

expect(res.statusCode).toEqual(400);
expect(res.text).toEqual('file-needs-upload');
});

it('returns 400 if the file has a new encryption key', async () => {
const fileId = crypto.randomBytes(16).toString('hex');
const groupId = 'group-id';
const keyId = 'old-key-id';
const newKeyId = 'new-key-id';
const syncVersion = 2;

// Add a mock file with the old key
addMockFile(fileId, groupId, keyId, JSON.stringify({ keyId }), syncVersion);

// Create a sync request with the new key
const syncRequest = createMinimalSyncRequest(fileId, groupId, newKeyId);
const res = await sendSyncRequest(syncRequest);

expect(res.statusCode).toEqual(400);
expect(res.text).toEqual('file-has-new-key');
});
});

function addMockFile(fileId, groupId, keyId, encryptMeta, syncVersion) {
getAccountDb().mutate(
'INSERT INTO files (id, group_id, encrypt_keyid, encrypt_meta, sync_version) VALUES (?, ?, ?,?, ?)',
[fileId, groupId, keyId, encryptMeta, syncVersion],
);
}

function createMinimalSyncRequest(fileId, groupId, keyId) {
const syncRequest = new SyncProtoBuf.SyncRequest();
syncRequest.setFileid(fileId);
syncRequest.setGroupid(groupId);
syncRequest.setKeyid(keyId);
syncRequest.setSince('2024-01-01T00:00:00.000Z');
syncRequest.setMessagesList([]);
return syncRequest;
}

async function sendSyncRequest(syncRequest) {
const serializedRequest = syncRequest.serializeBinary();
// Convert Uint8Array to Buffer
const bufferRequest = Buffer.from(serializedRequest);

const res = await request(app)
.post('/sync')
.set('x-actual-token', 'valid-token')
.set('Content-Type', 'application/actual-sync')
.send(bufferRequest);
return res;
}
3 changes: 2 additions & 1 deletion src/util/payee-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ function formatPayeeIban(iban) {
}

export const formatPayeeName = (trans) => {
const amount = trans.transactionAmount.amount;
const nameParts = [];

// get the correct name and account fields for the transaction amount
let name;
let account;
if (trans.amount > 0 || Object.is(Number(trans.amount), 0)) {
if (amount > 0 || Object.is(Number(amount), 0)) {
name = trans.debtorName;
account = trans.debtorAccount;
} else {
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/423.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [tcrasset]
---

Add integration tests for the /sync endpoint
6 changes: 6 additions & 0 deletions upcoming-release-notes/427.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Bugfix
authors: [matt-fidd]
---

Fix payee name selection based on the transaction amount
6 changes: 6 additions & 0 deletions upcoming-release-notes/430.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [MatissJanis, matt-fidd]
---

CI workflow for pinging PRs that have been in the "WIP" state for a week without an update.
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1709,13 +1709,13 @@ __metadata:
linkType: hard

"axios@npm:^1.2.1":
version: 1.7.2
resolution: "axios@npm:1.7.2"
version: 1.7.4
resolution: "axios@npm:1.7.4"
dependencies:
follow-redirects: "npm:^1.15.6"
form-data: "npm:^4.0.0"
proxy-from-env: "npm:^1.1.0"
checksum: 10c0/cbd47ce380fe045313364e740bb03b936420b8b5558c7ea36a4563db1258c658f05e40feb5ddd41f6633fdd96d37ac2a76f884dad599c5b0224b4c451b3fa7ae
checksum: 10c0/5ea1a93140ca1d49db25ef8e1bd8cfc59da6f9220159a944168860ad15a2743ea21c5df2967795acb15cbe81362f5b157fdebbea39d53117ca27658bab9f7f17
languageName: node
linkType: hard

Expand Down

0 comments on commit 7362262

Please sign in to comment.