-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add experimental fetch repetition field value implementation #3
base: main
Are you sure you want to change the base?
Conversation
add GenericCollectionFieldMetadata to handle '@MaxRepetitions', $Type 'Collection(Edm.String)' ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a preliminary review. I'll look over the FM documentation later to see if there is anything else missing.
@@ -238,8 +238,9 @@ class Connection { | |||
const cleanedJson = json.replace(/"(?:(?=(\\?))\1.)*?"/gs, substring => { | |||
return substring.replace(/(?<!\\)((?:\\\\)*)\n/g, '$1\\n'); | |||
}); | |||
const tabCleanedJson = cleanedJson.replace(/\t/g, '\\t'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an example response coming from FileMaker where this is an issue in the parsing process?
If it is, it should be handled in the regexp above to avoid escaping tabs which are outside of JSON strings.
|
||
public async fetchBinaryFieldValue(id : PrimaryKey, fieldName : string, repetition ?: number) : Promise<Blob> { | ||
let path : string; | ||
if (repetition === undefined) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always use curly braces.
describe('fetchFieldValue', () => { | ||
it('should return the result', async () => { | ||
databaseStub.fetchJson.returns(Promise.resolve({})); | ||
await table.fetchFieldValue('bar', 'baz'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing assertion on the return type.
); | ||
}); | ||
|
||
it('should return the result', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two tests have the same description – should be updated to clarify their difference.
No description provided.