-
Couldn't load subscription status.
- Fork 33
fix: enable history table in datasets lifecycle tab #2066
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
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/app/datasets/dataset-lifecycle/dataset-lifecycle.component.ts:69` </location>
<code_context>
const dataset = this.dataset;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {dataset} = this;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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, couple of questions
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
ObsoleteDatasetOutputDtowith history field), as support for legacy history was added in SciCatProject/scicat-backend-next#2282Motivation
Fixes:
Changes:
Tests included
Documentation
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
Screenshot
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:
Enhancements:
Tests: