Feat/1956/issue details last seen#1965
Conversation
| 'issueDetails.issueDetails': 'Issue Details', | ||
| 'issueDetails.issueListingInfo': | ||
| 'The culprit for all issues listed is code', | ||
| 'issueDetails.lastIncident': 'Last incident', |
There was a problem hiding this comment.
Just to keep the same pattern, we could go with 'Last Incident Data'
| issue_version?: string; | ||
| }; | ||
|
|
||
| export type LastIncident = { |
There was a problem hiding this comment.
Do we need a new type here? If there is no new data, we could just change to a type named Incident
| checkout_id: Optional[str] | ||
|
|
||
|
|
||
| class LastIncident(BaseModel): |
There was a problem hiding this comment.
Do we need to differentiate types for first and last incident?
There was a problem hiding this comment.
not exactly. Now the only difference are the "last_seen" and "first_seen" properties. But I agree we can have an unified Incident type with this data
| if len(issue_id_list) == 1: | ||
| comparison = "= %s" | ||
| else: | ||
| placeholders = ", ".join(["%s"] * len(issue_id_list)) |
There was a problem hiding this comment.
I think we dont need to do this trick here. We might be able to pass as a list parameter.
There is an example of the commits parameter (a list) on get_hardware_details_summary.
| """ | ||
| Assigns the last seen data to the processed_issues_table by querying with the issue_key_list. | ||
| """ | ||
| issue_id_set: set[str] = set() |
There was a problem hiding this comment.
just a nit: we might be able to get the same with issue_id_set = {*issue_key_list.keys()}
| return records | ||
|
|
||
|
|
||
| def get_issue_last_seen_data(*, issue_id_list: list[str]) -> list[dict]: |
There was a problem hiding this comment.
This can be factored out with get_issue_first_seen_data into a single function with a toggle for asc/desc
| class ExtraIssuesData(BaseModel): | ||
| first_incident: FirstIncident | ||
| first_incident: Incident | ||
| last_incident: Optional[Incident] = None |
There was a problem hiding this comment.
last incident should not be optional
There was a problem hiding this comment.
last incident should not be optional
Agreed. It was optional before because ExtraIssuesData was being instantiated without last_incident, which was assigned later on. I changed backend/kernelCI_app/helpers/issueExtras.py to assign last_incident from the start.
| }, | ||
| ]; | ||
|
|
||
| export const getFirstIncidentSection = ({ |
There was a problem hiding this comment.
lingering "First" in name - this gets first and last incidents
| 'issueDetails.issueDetails': 'Issue Details', | ||
| 'issueDetails.issueListingInfo': | ||
| 'The culprit for all issues listed is code', | ||
| 'issueDetails.lastIncident': 'Last Incident Data', |
There was a problem hiding this comment.
nit: I prefer "Latest Incident", as in "the issue might or might not be finished"
| 'hardwareListing.treeSelectorSearchPlaceholder': 'Search tree...', | ||
| 'issue.alsoPresentTooltip': 'Issue also present in {tree}', | ||
| 'issue.firstSeen': 'First seen', | ||
| 'issue.lastSeen': 'Last seen', |
There was a problem hiding this comment.
No need to specify "last" when we are inside a "last incident" section. Ditto for "first seen"
Part of kernelci#1956 Signed-off-by: Felipe <felipebergamin@profusion.mobi>
Closes kernelci#1956 Signed-off-by: Felipe <felipebergamin@profusion.mobi>
felipebergamin
force-pushed
the
feat/1956/issue-details-last-seen
branch
from
5e2d10a to
d2ea2cf
Compare
3 participants
Closes issue #1956
Example of how it looks: