-
Couldn't load subscription status.
- Fork 32
feat: add backwards compatible history to v3/datasets response #2282
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
Conversation
3cff98a to
542320d
Compare
542320d to
94afb49
Compare
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.
thanks! minor comments, LGTM otherwise.
I think a class transformer would have been better suited by I see that we are already using these convertors throughout the code
src/datasets/utils/history.util.ts
Outdated
|
|
||
| const IGNORE_FIELDS = ["updatedAt", "updatedBy", "_id"]; | ||
|
|
||
| export function convertObsoleteHistoryToGenericHistory( |
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.
is this used anywhere?
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 will be used in the up migration script: afbb56d, creating a PR soon
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.
Migration script PR: #2286
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.
could you please move it in the migration PR then? I am not sure I agree too much with the migration one, but we could merge this one in the meantime if this was part of the other PR
src/datasets/utils/history.util.ts
Outdated
|
|
||
| const IGNORE_FIELDS = ["updatedAt", "updatedBy", "_id"]; | ||
|
|
||
| export function convertObsoleteHistoryToGenericHistory( |
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.
could you please move it in the migration PR then? I am not sure I agree too much with the migration one, but we could merge this one in the meantime if this was part of the other PR
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.
LGTM, thanks!
<!-- Follow semantic-release guidelines for the PR title, which is used in the changelog. Title should follow the format `<type>(<scope>): <subject>`, where - Type is one of: build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test|BREAKING CHANGE - Scope (optional) describes the place of the change (eg a particular milestone) and is usually omitted - subject should be a non-capitalized one-line description in present imperative tense and not ending with a period See https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines for more details. --> ## Description <!-- Short description of the pull request --> Creating a release to generate typescript sdk artifiacts, as `OutputDatasetObsoleteDto` and `HistoryClass` (used in the frontend) has changed (OutputDatasetObsoleteDto includes a v3 compatible history field, and HistoryClass includes an index signature #2282) ## Motivation <!-- Background on use case, changes needed --> ## Fixes <!-- Please provide a list of the issues fixed by this PR --> * Bug fixed (#X) ## Changes: <!-- Please provide a list of the changes implemented by this PR --> * changes made ## Tests included - [ ] Included for each change/fix? - [ ] Passing? <!-- Merge will not be approved unless tests pass --> ## Documentation - [ ] swagger documentation updated (required for API changes) - [ ] official documentation updated ### official documentation info <!-- If you have updated the official documentation, please provide PR # and URL of the updated pages -->
## Description As the new backend did not implemet history[] in /v3/datasets, history was turned off / commented out in #2041. Uncommented and fixed type errors (some errors are automatically fixed by using the v4.24.5 of scicat-sdk-ts-angular which includes `ObsoleteDatasetOutputDto` with history field), as support for legacy history was added in SciCatProject/scicat-backend-next#2282 ## Motivation ## Fixes: ## Changes: ## Tests included - [x] Included for each change/fix? - [x] Passing? (Merge will not be approved unless this is checked) ## Documentation - [ ] swagger documentation updated \[required\] - [ ] official documentation updated \[nice-to-have\] ### official documentation info If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included ## Backend version - [x] Does it require a specific version of the backend - which version of the backend is required: v4.24.5 ## Screenshot <img width="819" height="565" alt="history" src="https://github.com/user-attachments/assets/d06379a2-e137-4f70-a8eb-0367c49971bb" /> ## Summary by Sourcery Enable dataset history in the lifecycle tab by updating the SDK version, adding a custom type for history properties, reactivating the parseHistoryItems method, and updating the corresponding unit test. New Features: - Restore and display dataset history entries in the lifecycle tab Enhancements: - Upgrade scicat-sdk-ts-angular to v4.25.0 to include history in ObsoleteDatasetOutputDto - Add HistoryWithProperties type to support dynamic history fields in parsing logic Tests: - Un-comment and adapt the unit test for parseHistoryItems in DatasetLifecycleComponent
Description
Add history array to v3/datasets response, with the same format as backend-v3. The main difference between GenericHistory and HistoryClass format is that in the former, changed fields are grouped into
beforeandafter. Whereas in the latter, each field is present at the top level withfield: {previousValue: ..., currentValue: ...}.Another difference is with
Deltastrategy, the current history implementation only records fields that are acutally changed. Whereas in old backend, forDataset.datasetlifecycle, all fields from previous value of datasetlifecyle were recorded inpreviousValue.Motivation
Archive/retrieve workflows at PSI depend on backend-v3 representation of History
Fixes
dataset.servicefromDatasetClasstoDatasetDocumentChanges:
GenericHistorytoHistoryClass(GenericHistory[], Dataset)->HistoryClass[]that preserves older datasetlifecycle inpreviousValueTests included
Documentation
OutputDatasetObsoleteDtoto include thehistory[]array.Sample output
GET /datasets/{pid}
{ ... "principalInvestigator": "[email protected]", "history": [ { "updatedAt": "2025-10-14T08:05:10.202Z", "updatedBy": "ingestor", "_id": "", "keywords": { "previousValue": [ "new", "keyword" ], "currentValue": [ "new", "keyword", "trois" ] }, "description": { "previousValue": "This is a test description just for change", "currentValue": "one last test description" } }, { "updatedAt": "2025-10-14T07:37:04.757Z", "updatedBy": "ingestor", "_id": "", "isPublished": { "previousValue": true, "currentValue": false } }, { "updatedAt": "2025-10-14T07:23:28.580Z", "updatedBy": "ingestor", "_id": "", "keywords": { "previousValue": [], "currentValue": [ "new", "keyword" ] }, "description": { "previousValue": "This is a test description what noooo", "currentValue": "This is a test description just for change" } }, { "updatedAt": "2025-10-14T07:15:34.684Z", "updatedBy": "ingestor", "_id": "", "description": { "previousValue": "This is a test description back on v3-history", "currentValue": "This is a test description what noooo" } }, { "updatedAt": "2025-10-14T07:15:24.809Z", "updatedBy": "ingestor", "_id": "" }, { "updatedAt": "2025-10-14T07:00:24.262Z", "updatedBy": "ingestor", "_id": "", "keywords": { "previousValue": [ "newtag" ], "currentValue": [] }, "description": { "previousValue": "This is a test description hello HEAD^2", "currentValue": "This is a test description back on v3-history" } }, { "updatedAt": "2025-10-14T06:57:00.237Z", "updatedBy": "ingestor", "_id": "", "keywords": { "previousValue": [], "currentValue": [ "newtag" ] } }, { "updatedAt": "2025-10-14T06:56:44.914Z", "updatedBy": "ingestor", "_id": "", "description": { "previousValue": "This is a test description hello", "currentValue": "This is a test description hello HEAD^2" } }, { "updatedAt": "2025-10-13T13:59:47.634Z", "updatedBy": "ingestor", "_id": "" }, { "updatedAt": "2025-10-13T13:59:17.508Z", "updatedBy": "ingestor", "_id": "", "description": { "previousValue": "This is a test description hola", "currentValue": "This is a test description hello" } }, { "updatedAt": "2025-10-13T13:57:50.173Z", "updatedBy": "ingestor", "_id": "", "description": { "previousValue": "This is a test description hello", "currentValue": "This is a test description hola" } } ] }