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.

Sign up for GitHub

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

update last read time for empty report task #43665

Merged
merged 5 commits into from
Jun 25, 2024
Merged

update last read time for empty report task #43665

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f07abfe
update condition compare by date instead of string
suneox Jun 13, 2024
991e265
clean up compare condition
suneox Jun 13, 2024
58e448a
update unread condition by task report type
suneox Jun 14, 2024
15a01d6
update lastReadTime for task report
suneox Jun 15, 2024
5dbb17a
explain reason update last read time
suneox Jun 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
explain reason update last read time
  • Loading branch information
@suneox
suneox committed Jun 23, 2024
commit 5dbb17a141f8c55ae3d736436c25d2034d6372d2
1 change: 1 addition & 0 deletions src/pages/home/ReportScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ function ReportScreen({
if (!!report.lastReadTime || !ReportUtils.isTaskReport(report)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why did we add !ReportUtils.isTaskReport(report)? and are you sure we're fixing the root cause?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

!ReportUtils.isTaskReport(report) this condition to make sure only update readNewestAction for task report to avoid regression

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we can remove the condition !ReportUtils.isTaskReport(report). I added this condition to ensure the fix only addresses the scope of this issue and avoids regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

@suneox it might fix similar issues for different kinds of reports wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, I don't found another use case the same with this issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@getusha i think we can go with this change only fix for an use-case task report, to avoid regression

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add a comment about the fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added comment explain reason below

return;
}
// After creating the task report then navigating to task detail we don't have any report actions and the last read time is empty so We need to update the initial last read time when opening the task report detail.
Report.readNewestAction(report.reportID);
}, [report]);

Expand Down
Loading