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

New version apis #13

Merged
merged 3 commits into from
Aug 24, 2017
Merged

New version apis #13

merged 3 commits into from
Aug 24, 2017

Conversation

jpalawaga
Copy link
Contributor

@jpalawaga jpalawaga commented Aug 10, 2017

For some reason my old PR stopped updating when I pushed new commits onto the branch :?

anyhow this has a few changes:

  • Fixes one broken test--the old attachment id appears to be marked as deleted
  • adds support for getting version sets for a project
  • adds support to get sheets based on a version set

and, as in the other pr, i'm working with the customer api team to get the api break fixed for the broken test.


This change is Reviewable

@kirkplangrid
Copy link
Contributor

Looks good, but there seems to be a test failure?

https://ci.appveyor.com/project/kirkplangrid/plangrid-api-net/build/2.0.8-uabagrmf

1) Test Error : PlanGrid.Api.Tests.AttachmentTests.UploadPdfAttachment
   Refit.ApiException : Response status code does not indicate success: 500 (INTERNAL SERVER ERROR).
   at Refit.RequestBuilderImplementation.<>c__DisplayClass2e`1.<<buildCancellableT

Any thoughts?


Reviewed 8 of 8 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@jpalawaga
Copy link
Contributor Author

Yeah... unforutnately when attachments were migrated from mongo to postgres, it seems as though CAPI had some bug in it. These integration tests are actually what caused that bug to surface.

I've asked the documents team to fix the breakage, but the tests will unforunately be broken until they do :sadpanda:


Comments from Reviewable

@jpalawaga
Copy link
Contributor Author

@kirkplangrid good to merge now?

@kirkplangrid
Copy link
Contributor

Hey, sorry, lost track of this. Reviewing now, but will get back to you later this morning.

@kirkplangrid kirkplangrid merged commit 45c65f8 into master Aug 24, 2017
@jpalawaga jpalawaga deleted the new-version-apis branch August 24, 2017 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants