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

Support getting attachments #3

Merged
merged 3 commits into from
May 2, 2023
Merged

Support getting attachments #3

merged 3 commits into from
May 2, 2023

Conversation

quinnturner
Copy link

@quinnturner quinnturner commented Apr 26, 2023

This has been tested locally by linking it in @sit/api and writing several unit tests.

I don't know what we want to do with the versioning. I could bump it normally. One idea is to try and track closely with Intacct's version, then add some version suffix.

For example, if we update this fork to include the latest code on Intacct's version 2.2.1, then we'd have something like: 2.2.1-fork.0, or something like that. That way, we can keep track if we are missing code on the base.

Alternatively, we can try and work towards merging more of our forked additions back into Intacct.

@quinnturner quinnturner marked this pull request as ready for review May 1, 2023 17:34
@quinnturner quinnturner requested a review from rudymalhi May 1, 2023 17:37
@@ -1,12 +1,12 @@
{
"name": "@intacct-thyme/intacct-sdk",
"version": "2.2.0",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a (minor) bug on latest. npm i needs to be run when bumping package.json when using NPM's package-lock.json.

import IaXmlWriter from "../../Xml/IaXmlWriter";
import AbstractFunction from "../AbstractFunction";

export default class AttachmentGet extends AbstractFunction {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All other Attachment* functions extends AbstractAttachment. I decided not to do that because it's somewhat incorrect, as not all properties are defined.

@quinnturner
Copy link
Author

I did my part by creating an associated PR for this feature on Intacct: intacct#117

@quinnturner quinnturner merged commit 4dba683 into master May 2, 2023
@quinnturner quinnturner deleted the attachment branch May 2, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants