search

freshbooks / freshbooks-nodejs-sdk

Node.js SDK for the FreshBooks API
MIT License
8 stars 12 forks source link

Add SortQueryBuilder and tests #1009

Closed mcse3010 closed 2 months ago

mcse3010 commented 2 months ago

Purpose

Adds SortQueryBuilder and tests

Related Material

See Issue https://github.com/freshbooks/freshbooks-nodejs-sdk/issues/1008

mcse3010 commented 2 months ago

Missed an export

mcse3010 commented 2 months ago

Didn't add docs as it looks auto-generated. Let me know if I need to do anything to help with the docs please.

mcse3010 commented 2 months ago

Will fix!

From: Andrew McIntosh @.> Sent: Tuesday, September 10, 2024 12:01:10 PM To: freshbooks/freshbooks-nodejs-sdk @.> Cc: Chadwick Posey @.>; State change @.> Subject: Re: [freshbooks/freshbooks-nodejs-sdk] Add SortQueryBuilder and tests (PR #1009)

@amcintosh commented on this pull request.

In packages/api/src/models/builders/SortQueryBuilder.tshttps://github.com/freshbooks/freshbooks-nodejs-sdk/pull/1009#discussion_r1752251052:

  • desc(key: string): SortQueryBuilder {
  • this.queryParams.push(new SortQueryParam(key, 'desc'))
  • return this
  • }
  • descending(key: string): SortQueryBuilder {
  • return this.desc(key)
  • }
  • build(resourceType?: string): string {
  • let isAccountingLike = false
  • if (!resourceType || ['AccountingResource', 'EventsResource'].includes(resourceType)) {
  • isAccountingLike = true
  • }
  • let queryString = ''
  • this.queryParams.forEach((param) => {

Hi. It looks like the API only supports a single sort parameter, all others are ignored. So I think this can be changed to just a single queryParam value that gets set with any asc or desc calls and no need to iterate.

— Reply to this email directly, view it on GitHubhttps://github.com/freshbooks/freshbooks-nodejs-sdk/pull/1009#pullrequestreview-2292960447, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABCMB4TQMKBGZUR3J7VNMSDZV4JUNAVCNFSM6AAAAABN64JID6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOJSHE3DANBUG4. You are receiving this because you modified the open/close state.Message ID: @.***>

amcintosh commented 2 months ago

Looks great. Thank you! I'll get it merged and do a new release.

amcintosh commented 2 months ago

New release 4.1.0

mcse3010 commented 2 months ago

Good afternoon Andrew,

Are you aware if there is any plans to support Timer functionality in the sdk? I know it’s not documented in the API docs, and I don’t know how much you’re responsible for that, but I do know the API supports it…

Thoughts?

Thanks Chadwick

From: Andrew McIntosh @.> Sent: Wednesday, September 11, 2024 1:13 PM To: freshbooks/freshbooks-nodejs-sdk @.> Cc: Chadwick Posey @.>; State change @.> Subject: Re: [freshbooks/freshbooks-nodejs-sdk] Add SortQueryBuilder and tests (PR #1009)

New release 4.1.0https://github.com/freshbooks/freshbooks-nodejs-sdk/releases/tag/%40freshbooks%2Fapi%404.1.0

— Reply to this email directly, view it on GitHubhttps://github.com/freshbooks/freshbooks-nodejs-sdk/pull/1009#issuecomment-2344246272, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABCMB4TV7L5J5TW4CEERS4TZWB2Y7AVCNFSM6AAAAABN64JID6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBUGI2DMMRXGI. You are receiving this because you modified the open/close state.Message ID: @.**@.>>

amcintosh commented 1 month ago

Hi @mcse3010. I don't know of any immediate plans from the team to document the timer API. I don't think we're opposed to including it though, so I can probably take a look in the next week or two.

mcse3010 commented 1 month ago

No worries, i just noticed all the models are there… it uses the comments endpoint to do its magic.

From: Andrew McIntosh @.> Sent: Monday, September 16, 2024 12:18:02 PM To: freshbooks/freshbooks-nodejs-sdk @.> Cc: Chadwick Posey @.>; Mention @.> Subject: Re: [freshbooks/freshbooks-nodejs-sdk] Add SortQueryBuilder and tests (PR #1009)

Hi @mcse3010https://github.com/mcse3010. I don't know of any immediate plans from the team to document the timer API. I don't think we're opposed to including it though, so I can probably take a look in the next week or two.

— Reply to this email directly, view it on GitHubhttps://github.com/freshbooks/freshbooks-nodejs-sdk/pull/1009#issuecomment-2353362702, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABCMB4RNEDXFZ2U3QUJZWUDZW4ADVAVCNFSM6AAAAABN64JID6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJTGM3DENZQGI. You are receiving this because you were mentioned.Message ID: @.***>