PLT-7646 Fix progress % values for properties in the Timeline-View grid #21
Conversation
…iew grid when Finished
WalkthroughThis comprehensive update focuses on enhancing the user interface, improving performance, and refining error handling. Key changes include consolidation of font styles, simplification of routing structure, introduction of new components like Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (3)
- src/pages/certification/Certification.helper.tsx (2 hunks)
- src/pages/certification/certification-result/CertificationResult.tsx (5 hunks)
- src/pages/certification/components/TimelineView/TimelineView.tsx (10 hunks)
Files skipped from review due to trivial changes (1)
- src/pages/certification/Certification.helper.tsx
Additional comments (Suppressed): 15
src/pages/certification/certification-result/CertificationResult.tsx (5)
1-4: The
useRefhook has been added to the import statement from 'react'. Ensure that this new hook is being used correctly in the component.21-27: A new helper function
isTaskSuccessand a new constanttaskKeyshave been imported fromCertification.helper. Make sure these are defined and exported correctly in the helper file.32-38: Two new
useRefhooks have been introduced:propertyBasedTestProgressTotalandcurrentPropertyBasedTestProgress. These hooks are used to store mutable values which can persist across re-renders without causing additional renders when their values change. This seems to be an appropriate use ofuseRef.55-70: The logic for processing the result JSON has been updated. Now, it not only processes the finished JSON but also calculates the progress of property-based testing. The calculation is based on the success status of each task in the result JSON. This seems like a reasonable approach to calculate the progress, but ensure that the
isTaskSuccessfunction correctly determines the success status of a task.114-120: The
ProgressCardcomponent is now being rendered with dynamic values forcurrentValueandtotalValue, which are derived from the refscurrentPropertyBasedTestProgressandpropertyBasedTestProgressTotal. This is a good improvement as it allows the progress bar to reflect the actual progress of property-based testing.src/pages/certification/components/TimelineView/TimelineView.tsx (10)
1-4: The
useRefhook is imported from React but it's not used in the old hunk. Ensure that this new import is necessary and used appropriately in other parts of the code.15-23: The
ICertificationTaskandisTaskSuccessare newly imported fromCertification.helper. Make sure these imports are used correctly in the updated code.34-38: A new property
keyhas been added to thePlanObjinterface. Ensure that this change doesn't break any existing functionality wherePlanObjis used.66-70: The
useRefhook is used to createcurrentPropertyBasedTestProgress. This is a new addition to the component state. Make sure its usage aligns with the intended behavior of the component.91-104: The logic for setting
plannedTestingTaskshas been modified. The new logic includes a check forTaskConfigbefore returning an object. This is a good practice as it prevents potential errors ifTaskConfigis undefined.123-129: The function
recalculateTestTaskProgressis called when a task succeeds. This is a new addition to the code. Ensure that this function call doesn't introduce any unexpected side effects.137-141: The function
recalculateTestTaskProgressis defined here. It adds the name of the task tocurrentPropertyBasedTestProgressif it's not already present. This seems to be a part of the new logic for tracking progress.268-281: The
ProgressCardcomponent is now rendered with dynamic values forcurrentValueandtotalValue. This is a significant improvement over the commented-out static values in the old hunk.294-300: The
indexparameter is added to the map function. Ensure that this new parameter is used correctly in the updated code.307-328: The logic for rendering the progress of each task has been updated. The new logic includes checks for the run status and task progress, and renders different components based on these checks. This seems to be a part of the new logic for tracking progress.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- src/pages/certification/certification-result/fullReportTable.css (1 hunks)
Files skipped from review due to trivial changes (1)
- src/pages/certification/certification-result/fullReportTable.css
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files selected for processing (2)
- src/components/LogsView/LogsView.tsx (1 hunks)
- src/pages/certification/components/TimelineView/TimelineView.tsx (10 hunks)
Additional comments (Suppressed): 11
src/components/LogsView/LogsView.tsx (2)
12-19: The new prop
latestTestingProgressis introduced in the function signature. Ensure that all calls to this component throughout the codebase have been updated to match the new signature if necessary.70-78: This block of code introduces a new logic to handle the progress of property-based tests. It checks if the
latestTestingProgressfunction is provided, and if the current log entry is the second last one with "run-certify" in its source. If these conditions are met, it parses the text of the log entry as JSON and passes it to thelatestTestingProgressfunction. This seems like a specific use case, so ensure that it doesn't interfere with other uses of theLogsViewcomponent.src/pages/certification/components/TimelineView/TimelineView.tsx (9)
4-4: The
useRefhook is imported but not used in the old hunk. Ensure that it's necessary for the new code.15-21: The import statement has been expanded to include
ICertificationTaskandisTaskSuccess. Make sure these are being used in the new code.34-38: A new property
keyhas been added to thePlanObjinterface. Confirm that this change doesn't break any existing functionality wherePlanObjis used.66-67: The
currentPropertyBasedTestProgressstate variable has been introduced usinguseRef. This seems to be a new addition to manage the progress of the current property-based test.83-112: The
assignProgressToTasksfunction has been introduced to assign progress to tasks based on the received progress data. It updates theplannedTestingTasksstate with the calculated progress. This is a significant change from the previous logic, which directly updated theplannedTestingTasksstate inside thehandleTestingTimelineDetailsfunction.145-149: The
recalculateTestTaskProgressfunction has been introduced to update thecurrentPropertyBasedTestProgressref with the name of the completed task. This seems to be a new feature for tracking the progress of property-based tests.192-195: The
processLatestTestingProgressFromLogsfunction has been introduced to process the latest testing progress from logs and assign it to tasks. This seems to be a new feature for updating task progress based on log data.292-292: The
ProgressCardcomponent is now being used with dynamic values forcurrentValueandtotalValue. Ensure that these values are calculated correctly and the component displays the expected progress.311-312: The
indexparameter has been added to the map function. Make sure this doesn't break any existing functionality where the map function is used.
src/pages/certification/components/TimelineView/TimelineView.tsx
Outdated
Show resolved
Hide resolved
src/pages/certification/components/TimelineView/TimelineView.tsx
Outdated
Show resolved
Hide resolved
|
@RSoulatIOHK Resolved the calculations within the progress% in TimelineView ; and added the Property Based Testing progress card in both Timeline View and Full Report View when run is finished. Calculations are updated here - https://input-output.atlassian.net/browse/PLT-7646?focusedCommentId=217176 |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Files selected for processing (6)
- src/components/LogsView/LogsView.tsx (1 hunks)
- src/components/RepoAccessStatus/repoAccessStatus.config.ts (1 hunks)
- src/pages/certification/Certification.helper.tsx (3 hunks)
- src/pages/certification/certification-result/CertificationResult.tsx (5 hunks)
- src/pages/certification/components/AuditorRunTestForm/AuditorRunTestForm.tsx (6 hunks)
- src/pages/certification/components/TimelineView/TimelineView.tsx (8 hunks)
Files skipped from review due to trivial changes (2)
- src/components/RepoAccessStatus/repoAccessStatus.config.ts
- src/pages/certification/components/AuditorRunTestForm/AuditorRunTestForm.tsx
Additional comments (Suppressed): 16
src/pages/certification/certification-result/CertificationResult.tsx (5)
1-4: The
useRefhook is imported but not used in the old hunk. Ensure that this new import is necessary for the changes made in the new hunks.21-27: New functions and types are imported from
Certification.helper. Make sure these are implemented correctly and used as intended in the new code.32-39: A new state variable
testTaskProgressis introduced using theuseRefhook. This seems to be used for storing progress information of tasks.54-80: The logic for processing the result JSON has been significantly expanded. It now includes a loop over the keys of the result JSON, where each key is matched with a task configuration. If a match is found and the corresponding value in the result JSON is truthy, a new object is created and pushed into
testTaskProgress.current. This seems to be part of the new progress calculation logic.124-134: The
ProgressCardcomponent is now rendered with dynamic values forcurrentValueandtotalValue, calculated by the newly imported functionscalculateCurrentProgressandcalculateExpectedProgress. This should provide a more accurate representation of the progress based on the actual data.- {/* <ProgressCard title={"Property Based Testing"} currentValue={100} totalValue={1000} /> */} + <ProgressCard + title={"Property Based Testing"} + currentValue={calculateCurrentProgress(testTaskProgress.current)} + totalValue={calculateExpectedProgress(testTaskProgress.current)} + />src/pages/certification/Certification.helper.tsx (2)
1-53: The new hunk introduces several interfaces to define the structure of certification tasks, testing progress, and planned tasks. This is a good practice as it improves code readability and maintainability by providing clear definitions of the data structures used in the code. The
CertificationTasksarray has been updated to use the newly introducedICertificationTaskinterface.152-176: New utility functions
parseTestCount,calculateCurrentProgress, andcalculateExpectedProgresshave been added. These functions seem to be well-implemented and should improve the modularity and readability of the code.src/pages/certification/components/TimelineView/TimelineView.tsx (8)
1-4: The
useRefhook has been added to the import statement from React. Ensure that this new hook is used appropriately in the code and that its implications are understood.14-29: Several new imports have been added, including
calculateCurrentProgress,calculateExpectedProgress,getProgressCardInfo,ICertificationTask,IFinishedTestingTaskDetails,ITestingProgress,PlanObj, andisTaskSuccess. Make sure these new imports are used correctly and efficiently in the code.32-40: A new component
StatusIconhas been imported and two new constantsPROGRESS_PASSandPROGRESS_FAILhave been defined. Ensure that these changes align with the intended functionality of the code.89-144: The logic for setting planned testing tasks has been significantly updated. The new logic seems more comprehensive and handles different scenarios more accurately. However, it's important to verify that these changes don't introduce any unexpected behavior or bugs.
- item.progress = Math.trunc( - ((currentProgressStats["successes"] + - currentProgressStats["failures"]) / - currentProgressStats["expected"]) * - 100 - ); + item.progress = currentProgressStats["expected"] ? Math.trunc( + ((currentProgressStats["successes"] + currentProgressStats["failures"]) / + (currentProgressStats["expected"] - currentProgressStats["discarded"])) * 100 + ) : "On Going";In the old hunk, the progress was calculated as a percentage of successes and failures over expected. In the new hunk, discarded tests are subtracted from the expected total before calculating the percentage. This seems to be a more accurate way of calculating progress, but it's important to verify that this change doesn't have any unintended side effects.
183-205: A new function
processLatestTestingProgressFromLogshas been introduced. This function updates the planned testing tasks based on the latest testing progress from logs. It appears to handle different scenarios correctly, but it's important to verify that it works as expected in all cases.207-220: The
getTaskProgressfunction has been introduced to determine the progress of each task. It uses theStatusIconcomponent to display the status of each task. Ensure that this function is working as expected and that it accurately reflects the status of each task.302-324: The
FileCoverageContainerandProgressCardcomponents are now conditionally rendered based on whether the run status is "finished". ThecalculateCurrentProgressandcalculateExpectedProgressfunctions are used to calculate the current and total values for theProgressCardcomponent. Make sure these changes work as expected and improve the user experience.- {/* <ProgressCard title={"Property Based Testing"} currentValue={100} totalValue={1000}/> */} + <ProgressCard + title={"Property Based Testing"} + currentValue={calculateCurrentProgress(plannedTestingTasks)} + totalValue={calculateExpectedProgress(plannedTestingTasks)} + />
- 338-344: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [338-357]
The logic for displaying task progress in the table has been updated. Instead of using an image to indicate complete progress, the
getTaskProgressfunction is now used. This should provide a more consistent and flexible way of displaying task progress.- {task.progress === 100 ? ( - <img - className="image" - src="/images/status-check.svg" - alt="complete" - /> - ) : ( - <span>{task.progress}%</span> - )} + {getTaskProgress(task, index)}src/components/LogsView/LogsView.tsx (1)
- 7-13: The
LogsViewcomponent now accepts an additional proplatestTestingProgress. This function is expected to be called with the second last log entry when polling ends and the source of the log entry contains 'run-certify'. Ensure that this new prop is correctly passed wherever this component is used.
src/pages/certification/certification-result/CertificationResult.tsx
Outdated
Show resolved
Hide resolved
https://input-output.atlassian.net/browse/PLT-7646
BE PR input-output-hk/dapps-certification#90
Summary by CodeRabbit
CircularProgressBarandProgressCardcomponents for better progress visualization.RepoAccessStatuscomponent to display repository access status.StatusIconcomponent for displaying various status icons.App.tsxand removed unused imports and components.Buttoncomponent to support passing an icon element.TextAreacomponent with onBlur validation and floating label functionality.TableComponentto support collapsible rows and show/hide column functionality.ColVizcomponent for better visual presentation and interaction.useLocalStoragehook for better error handling and value type flexibility.repoAccessStatus.config.ts.LogsViewcomponent.