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

Add YTD and last year to Reports headers #4019

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+79 −0
Open

Add YTD and last year to Reports headers #4019

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
97e5d90
Add YTD and last year to Reports headers
rodriguestiago0 Dec 21, 2024
08d75b9
Add release note
rodriguestiago0 Dec 21, 2024
3e1d7e7
Merge branch 'master' into add_ytd_and_last_year
rodriguestiago0 Dec 21, 2024
26c2b37
lint
rodriguestiago0 Dec 21, 2024
585d042
merge
rodriguestiago0 Dec 21, 2024
290058a
fix end date picker
rodriguestiago0 Dec 21, 2024
121d09b
remove debugger
rodriguestiago0 Dec 21, 2024
f4973c8
Update VRT
github-actions[bot] Dec 22, 2024
690627e
Merge branch 'master' into add_ytd_and_last_year
youngcw Dec 22, 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
48 changes: 48 additions & 0 deletions packages/desktop-client/src/components/reports/Header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type RuleConditionEntity,
type TimeFrame,
} from 'loot-core/types/models';
import { type SyncedPrefs } from 'loot-core/types/prefs';

import { Button } from '../common/Button2';
import { Select } from '../common/Select';
Expand All @@ -15,6 +16,7 @@ import { AppliedFilters } from '../filters/AppliedFilters';
import { FilterButton } from '../filters/FiltersMenu';
import { useResponsive } from '../responsive/ResponsiveProvider';

import { getLiveRange } from './getLiveRange';
import {
calculateTimeRange,
getFullRange,
Expand All @@ -29,6 +31,8 @@ type HeaderProps = {
mode?: TimeFrame['mode'];
show1Month?: boolean;
allMonths: Array<{ name: string; pretty: string }>;
earliestTransaction: string;
firstDayOfWeekIdx?: SyncedPrefs['firstDayOfWeekIdx'];
onChangeDates: (
start: TimeFrame['start'],
end: TimeFrame['end'],
Expand All @@ -51,6 +55,8 @@ export function Header({
mode,
show1Month,
allMonths,
earliestTransaction,
firstDayOfWeekIdx,
onChangeDates,
filters,
conditionsOp,
Expand All @@ -63,6 +69,14 @@ export function Header({
const { t } = useTranslation();
const { isNarrowWidth } = useResponsive();

function convertToMonth(
start: string,
end: string,
mode: TimeFrame['mode'],
): [string, string, TimeFrame['mode']] {
return [monthUtils.getMonth(start), monthUtils.getMonth(end), mode];
}
Comment on lines +72 to +78
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding input validation to convertToMonth.

The function assumes valid date strings but doesn't handle invalid inputs.

Add input validation:

 function convertToMonth(
   start: string,
   end: string,
   mode: TimeFrame['mode'],
 ): [string, string, TimeFrame['mode']] {
+  if (!start || !end) {
+    throw new Error('Invalid date strings provided');
+  }
   return [monthUtils.getMonth(start), monthUtils.getMonth(end), mode];
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function convertToMonth(
start: string,
end: string,
mode: TimeFrame['mode'],
): [string, string, TimeFrame['mode']] {
return [monthUtils.getMonth(start), monthUtils.getMonth(end), mode];
}
function convertToMonth(
start: string,
end: string,
mode: TimeFrame['mode'],
): [string, string, TimeFrame['mode']] {
if (!start || !end) {
throw new Error('Invalid date strings provided');
}
return [monthUtils.getMonth(start), monthUtils.getMonth(end), mode];
}


return (
<View
style={{
Expand Down Expand Up @@ -156,6 +170,40 @@ export function Header({
>
{t('1 Year')}
</Button>
<Button
variant="bare"
onPress={() =>
onChangeDates(
...convertToMonth(
...getLiveRange(
'Year to date',
earliestTransaction,
true,
firstDayOfWeekIdx,
),
),
)
}
>
{t('Year to date')}
</Button>
<Button
variant="bare"
onPress={() =>
onChangeDates(
...convertToMonth(
...getLiveRange(
'Last year',
earliestTransaction,
true,
firstDayOfWeekIdx,
),
),
)
}
>
{t('Last year')}
</Button>
<Button
variant="bare"
onPress={() =>
Expand Down
4 changes: 4 additions & 0 deletions packages/desktop-client/src/components/reports/reports/Calendar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,8 @@ function CalendarInner({ widget, parameters }: CalendarInnerProps) {
},
);

const [earliestTransaction, _] = useState('');

Comment on lines +463 to +464
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Initialize earliestTransaction state with data from useEffect

The earliestTransaction state is initialized but never updated. The component already has an effect that fetches the earliest transaction, which should be used to update this state.

- const [earliestTransaction, _] = useState('');
+ const [earliestTransaction, setEarliestTransaction] = useState('');

  useEffect(() => {
    async function run() {
      try {
        const trans = await send('get-earliest-transaction');
+       if (trans) {
+         setEarliestTransaction(trans.date);
+       }
        // ... rest of the effect
      } catch (error) {
        console.error('Error fetching earliest transaction:', error);
      }
    }
    run();
  }, []);

Committable suggestion skipped: line range outside the PR's diff.

return (
<Page
header={
Expand Down Expand Up @@ -492,6 +494,8 @@ function CalendarInner({ widget, parameters }: CalendarInnerProps) {
allMonths={allMonths}
start={start}
end={end}
earliestTransaction={earliestTransaction}
firstDayOfWeekIdx={firstDayOfWeekIdx}
mode={mode}
onChangeDates={onChangeDates}
filters={conditions}
Expand Down
7 changes: 7 additions & 0 deletions packages/desktop-client/src/components/reports/reports/CashFlow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {

import { useFilters } from '../../../hooks/useFilters';
import { useNavigate } from '../../../hooks/useNavigate';
import { useSyncedPref } from '../../../hooks/useSyncedPref';
import { theme } from '../../../style';
import { AlignedText } from '../../common/AlignedText';
import { Block } from '../../common/Block';
Expand Down Expand Up @@ -187,6 +188,10 @@ function CashFlowInner({ widget }: CashFlowInnerProps) {
});
};

const [earliestTransaction, _] = useState('');
const [_firstDayOfWeekIdx] = useSyncedPref('firstDayOfWeekIdx');
const firstDayOfWeekIdx = _firstDayOfWeekIdx || '0';

if (!allMonths || !data) {
return null;
}
Expand Down Expand Up @@ -224,6 +229,8 @@ function CashFlowInner({ widget }: CashFlowInnerProps) {
allMonths={allMonths}
start={start}
end={end}
earliestTransaction={earliestTransaction}
firstDayOfWeekIdx={firstDayOfWeekIdx}
mode={mode}
show1Month
onChangeDates={onChangeDates}
Expand Down
7 changes: 7 additions & 0 deletions packages/desktop-client/src/components/reports/reports/NetWorth.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { type TimeFrame, type NetWorthWidget } from 'loot-core/types/models';
import { useAccounts } from '../../../hooks/useAccounts';
import { useFilters } from '../../../hooks/useFilters';
import { useNavigate } from '../../../hooks/useNavigate';
import { useSyncedPref } from '../../../hooks/useSyncedPref';
import { theme, styles } from '../../../style';
import { Button } from '../../common/Button2';
import { Paragraph } from '../../common/Paragraph';
Expand Down Expand Up @@ -162,6 +163,10 @@ function NetWorthInner({ widget }: NetWorthInnerProps) {
});
};

const [earliestTransaction, _] = useState('');
const [_firstDayOfWeekIdx] = useSyncedPref('firstDayOfWeekIdx');
const firstDayOfWeekIdx = _firstDayOfWeekIdx || '0';

if (!allMonths || !data) {
return null;
}
Expand Down Expand Up @@ -197,6 +202,8 @@ function NetWorthInner({ widget }: NetWorthInnerProps) {
allMonths={allMonths}
start={start}
end={end}
earliestTransaction={earliestTransaction}
firstDayOfWeekIdx={firstDayOfWeekIdx}
mode={mode}
onChangeDates={onChangeDates}
filters={conditions}
Expand Down
7 changes: 7 additions & 0 deletions packages/desktop-client/src/components/reports/reports/Summary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {

import { useFilters } from '../../../hooks/useFilters';
import { useNavigate } from '../../../hooks/useNavigate';
import { useSyncedPref } from '../../../hooks/useSyncedPref';
import { SvgEquals } from '../../../icons/v1';
import { SvgCloseParenthesis } from '../../../icons/v2/CloseParenthesis';
import { SvgOpenParenthesis } from '../../../icons/v2/OpenParenthesis';
Expand Down Expand Up @@ -147,6 +148,10 @@ function SummaryInner({ widget }: SummaryInnerProps) {
}>
>([]);

const [earliestTransaction, _] = useState('');
const [_firstDayOfWeekIdx] = useSyncedPref('firstDayOfWeekIdx');
const firstDayOfWeekIdx = _firstDayOfWeekIdx || '0';
Comment on lines +151 to +153
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Initialize earliestTransaction state with data from useEffect

The earliestTransaction state is initialized but never updated, despite having an effect that fetches the earliest transaction. Consider updating the state within the effect.

- const [earliestTransaction, _] = useState('');
+ const [earliestTransaction, setEarliestTransaction] = useState('');

  useEffect(() => {
    async function run() {
      const trans = await send('get-earliest-transaction');
+     if (trans) {
+       setEarliestTransaction(trans.date);
+     }
      // ... rest of the effect
    }
    run();
  }, []);

Committable suggestion skipped: line range outside the PR's diff.


useEffect(() => {
async function run() {
const trans = await send('get-earliest-transaction');
Expand Down Expand Up @@ -273,6 +278,8 @@ function SummaryInner({ widget }: SummaryInnerProps) {
allMonths={allMonths}
start={start}
end={end}
earliestTransaction={earliestTransaction}
firstDayOfWeekIdx={firstDayOfWeekIdx}
mode={mode}
onChangeDates={onChangeDates}
onApply={dividendFilters.onApply}
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/4019.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Enhancements
authors: [rodriguestiago0]
---

Add "Year to date" and "Last year" to reports header.
Loading