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

fix(api-v2): Fix incorrect serialisation of lastModificationDate #1442

Merged
merged 28 commits into from
Jan 21, 2020

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Sep 18, 2019

This fixes the serialisation of knora-api:lastModificationDate in ontology requests/responses.

Clients that use the ontology API will need to update.

Fixes #1439.
Needs dasch-swiss/knora-ui#324.
Needs dasch-swiss/dsp-tools#11.
Needs dasch-swiss/dsp-js-lib#47.

@tobiasschweizer
Copy link
Contributor

@benjamingeer I will fix the anything ontology's last mod. date on this branch

@tobiasschweizer
Copy link
Contributor

dasch-swiss/dsp-js-lib#58 fixes the parsing of the lastModficationDate for knora-api-js-lib.

@benjamingeer
Copy link
Author

@tobiasschweizer Is this OK to merge? Lukas said we don't need to wait for knora-py.

@tobiasschweizer
Copy link
Contributor

I think it would be good to merge #1577 first. With that PR, the tests will only pass if dasch-swiss/dsp-js-lib#58 is implemented correctly.

This is because #1577 automatically generates client code and test data from the Knora client api route, integrates it into knora-api-js-lib and runs the unit tests with the generated code and test data.

@tobiasschweizer
Copy link
Contributor

In this case, however, this would mean that the correct branch of knora-api-js-lib has to be used with this PR.

@benjamingeer
Copy link
Author

I think it would be good to merge #1577 first. With that PR, the tests will only pass if dasch-swiss/dsp-js-lib#58 is implemented correctly.

This is because #1577 automatically generates client code and test data from the Knora client api route, integrates it into knora-api-js-lib and runs the unit tests with the generated code and test data.

I don't understand. This PR only affects API v2, so it doesn't affect any generated code.

@tobiasschweizer
Copy link
Contributor

This PR only affects API v2, so it doesn't affect any generated code.

It is just to make sure that knora-api-js-lib is actually in sync with knora-api

@benjamingeer
Copy link
Author

It is just to make sure that knora-api-js-lib is actually in sync with knora-api

OK. Anyway it looks like one of the tests is still broken, I'll look at it tomorrow.

@benjamingeer
Copy link
Author

Test wasn't broken, I reran it on GitHub and it worked this time.

@tobiasschweizer
Copy link
Contributor

Will look at this tomorrow too. I would be glad to get rid of these open PRs. Open for too long ...

@benjamingeer
Copy link
Author

benjamingeer commented Jan 20, 2020

Open for too long ...

That's why I want to generate as much client code as possible.

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Jan 21, 2020

Ok, I am generating the client code from this PR and will integrate it into the tracking PR dasch-swiss/dsp-js-lib#58 to see if anything is missing.

@tobiasschweizer
Copy link
Contributor

@benjamingeer @subotic This PR will necessitate a new published version https://github.com/dasch-swiss/knora-api-js-lib to be used with the release of Knora this PR is integrated into.

Copy link
Contributor

@tobiasschweizer tobiasschweizer left a comment

Choose a reason for hiding this comment

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

Ok, looks good! Pleas review dasch-swiss/dsp-js-lib#58 and then this PR is good to go.

@benjamingeer
Copy link
Author

@tobiasschweizer Thanks for the review!

@benjamingeer benjamingeer merged commit 22727a2 into develop Jan 21, 2020
@benjamingeer benjamingeer deleted the wip/1439-last-modification-date branch January 21, 2020 14:39
@benjamingeer benjamingeer modified the milestones: 2019-09, 2020-01 Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON-LD Serialization of an xsd:dateTimeStamp
3 participants