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.

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

Default selected audit year not just datetime.now().year #460

Closed
anchit-chandran opened this issue Jan 4, 2025 · 6 comments · Fixed by #482 or #484
Closed

Default selected audit year not just datetime.now().year #460

anchit-chandran opened this issue Jan 4, 2025 · 6 comments · Fixed by #482 or #484
Assignees

Comments

@anchit-chandran
Copy link
Contributor

@eatyourpeas was looking to update this myself now but unsure if it's set / used in multiple places, and unsure if I'm right, so wanted to confirm with you

Currently session.py::65, in return of create_session_object(), we set the default selected_audit_year as:

"selected_audit_year": datetime.now().year,

But I think that because audit periods are laid out like:

        Audit Period	Audit period start	Audit period end
        2024 - 2025	        1-Apr-24	        31-Mar-25
        2025 - 2026	        1-Apr-25	        31-Mar-26
        2026 - 2027	        1-Apr-26	        31-Mar-27

the default audit year for e.g. 4 Jan 2025 ISNT 2025 but is 2024?

We have a get_audit_period_for_date() fn inside general_functions/audit_period.py that gets audit period for given date so simplest change could be like:

get_audit_period_for_date(datetime.now())[0].year 
@mbarton
Copy link
Member

mbarton commented Jan 8, 2025

We also need the audit selector to work for 23-24 as the plan is to validate the KPI calculations by re-uploading all the CSV files from the last audit year into the new platform.

Should I expand the scope of this issue to cover that or raise a separate one?

@anchit-chandran
Copy link
Contributor Author

That should be an easy fix, if we only use this get_audit_period_for_date() function (central place to get audit date periods)

Can just increase lower bound to work for 23-24, can't remember if it already does

@mbarton
Copy link
Member

mbarton commented Jan 13, 2025

Cool yeah the audit year selector is working for me again now (not sure what was wrong). So I've raised #482 to close off this issue and #481 to discuss how we'll add subsequent audit years

@mbarton mbarton reopened this Jan 13, 2025
@mbarton
Copy link
Member

mbarton commented Jan 13, 2025

Reopening as there's some other instances of datetime.now().year and date.today().year I didn't address in the first PR

@anchit-chandran
Copy link
Contributor Author

Yep this is what i wanted to confirm with @eatyourpeas whether adjusting the default setting would affect other things

@mbarton
Copy link
Member

mbarton commented Jan 14, 2025

They all need updating - at the moment on live if you upload a CSV you can’t see it because the CSV code is using date.today().year but the switcher is correctly using audit year.

PR incoming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants