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

Jump to bottom

Replace cohorts with organisations directly #2783

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Replace cohorts with organisations directly #2783

wants to merge 11 commits into from

Conversation

thomasleese
Copy link
Contributor

@thomasleese thomasleese commented Dec 19, 2024 •
edited
Loading

Following on from #2780 each patient now has a specific year group they belong to, so we don't need the cohort model which at the moment is just a duplicate of this information. Instead we can have a direct link between patients and organisations, simplifying quite a bit of the code related to cohorting.

See individual commits for more details, the business logic is the same before and after. This will allow us to remove the Cohort model entirely and simplify our database structure.

@thomasleese thomasleese added the 🛠️ refactor Improving maintainability label Dec 19, 2024
@thomasleese thomasleese added this to the v1.1.3 milestone Dec 19, 2024
@thomasleese thomasleese added the 📐 data_consistency Validations and data tweaks label Dec 19, 2024
@thomasleese thomasleese force-pushed the cohort-year-group branch 2 times, most recently from 32e68cd to 8396a23 Compare December 20, 2024 11:28
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch from 08af78b to e9533b0 Compare December 20, 2024 11:28
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch from e9533b0 to f76bbf6 Compare December 20, 2024 14:35
@tvararu tvararu temporarily deployed to mavis-pr-2783 December 20, 2024 14:36 Inactive
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch from f76bbf6 to e0ed571 Compare January 5, 2025 10:59
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch 2 times, most recently from 2b320bd to e8dfb1b Compare January 7, 2025 13:49
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch from e8dfb1b to e75047f Compare January 8, 2025 11:30
Base automatically changed from cohort-year-group to main January 8, 2025 12:17
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch 2 times, most recently from cd002d6 to bc17344 Compare January 8, 2025 14:37
thomasleese added a commit that referenced this pull request Jan 8, 2025
@thomasleese
Avoid duplicate DATE_OF_BIRTH errors
0ca00ea 
This fixes a few minor visual issues spotted by the testers when testing
#2780 where
if the date of birth is missing the same error will appear twice for
each row, and similarly an issue where the year group wouldn't render
correctly.

This fixes are already in place for a future release (in
#2783 and
#2784),
however including them now makes the testing easier and avoids needing
to record additional defects.
@thomasleese thomasleese mentioned this pull request Jan 8, 2025
Merged
thomasleese added a commit that referenced this pull request Jan 8, 2025
@thomasleese
Avoid duplicate DATE_OF_BIRTH errors
0130357 
This fixes a few minor visual issues spotted by the testers when testing
#2780 where
if the date of birth is missing the same error will appear twice for
each row, and similarly an issue where the year group wouldn't render
correctly.

This fixes are already in place for a future release (in
#2783 and
#2784),
however including them now makes the testing easier and avoids needing
to record additional defects.
thomasleese added a commit that referenced this pull request Jan 8, 2025
@thomasleese
Avoid duplicate DATE_OF_BIRTH errors (#2826)
b284af2 
This fixes a few minor visual issues spotted by the testers when testing
#2780 where
if the date of birth is missing the same error will appear twice for
each row, and similarly an issue where the year group wouldn't render
correctly.

This fixes are already in place for a future release (in
#2783 and
#2784),
however including them now makes the testing easier and avoids needing
to record additional defects.

## Screenshots


![{CFD5D2F8-A8E7-40F1-B323-45361B94AB3A}](https://github.com/user-attachments/assets/fa2a0a2b-9ad0-4913-a71f-ea48ae7c82cf)

![{9A759A33-DF06-4FCC-9F59-9B9C8FF22271}](https://github.com/user-attachments/assets/eef016c9-47ad-4b14-8ced-5e2dd8e19427)
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch 2 times, most recently from 91a9062 to 7b7939f Compare January 10, 2025 11:45
@thomasleese thomasleese modified the milestones: v1.3.0, v1.4.0 Jan 10, 2025
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch from 7b7939f to d5304a1 Compare January 10, 2025 12:34
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch 3 times, most recently from 42701d1 to 500e67b Compare January 13, 2025 17:29
@thomasleese thomasleese marked this pull request as ready for review January 13, 2025 17:32
@thomasleese
Add organisation reference
60629fe 
This adds a new reference to the organisation on the patient model which
adds a direct link between a patient and an organisation rather than
going through the cohort which is going to be removed.
@thomasleese
Set organisation reference
124626d 
This updates everywhere that sets the cohort on the patient to also set
the organisation so we can start to use this instead of the cohort.
@thomasleese
Update PatientPolicy
b29239c 
This updates the patient policy to select children according to the
organisation they belong to rather than the cohort as the cohort is
going to be replaced with the organisation.
@thomasleese
Update AppPatientCohortTableComponent
d38c80d 
This updates the component to display the cohort from the organisation
as the cohort is going to be removed.
@thomasleese
Update Patient#add_to_upcoming_sessions
fa33c9e
@thomasleese
Add Integer#to_year_group
5de4fb3 
This adds a convenience method which allows us to easily convert between
a year group and a birth academic year.
@thomasleese
Update cohorts page
a71bf21 
This updates the cohorts page to not rely on the `Cohort` instances and
instead go by the `year_group` on the patients.
@thomasleese
Update schools moves to not set cohort
863dce4 
This updates the school moves to no longer set the cohort on the patient
as this is no longer used and instead we will rely on only the
organisation.
@thomasleese
Add Patient#in_programme
b48ff55 
This adds a scope on the patient model which allows us to filter on
patients who are in the programme based on their year group.
@thomasleese
Don't eager load cohorts
53e33b6 
This avoids eager loading any cohorts as we're going to stop using this
column and it will be shortly removed.
@thomasleese
Remove cohort reference
91c7d09 
This removes the cohort reference on patients, and removes any final
bits of code that rely on the cohort reference.
@thomasleese thomasleese force-pushed the replace-cohort-with-organisation branch from 500e67b to 91c7d09 Compare January 14, 2025 17:22
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tvararu tvararu tvararu approved these changes

Assignees
No one assigned
Labels
📐 data_consistency Validations and data tweaks 🛠️ refactor Improving maintainability
Projects
None yet
Milestone
v1.4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@thomasleese @tvararu