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

Schools #641

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

Schools #641

wants to merge 65 commits into from

Conversation

Rotheem
Copy link
Contributor

@Rotheem Rotheem commented Dec 22, 2024

Description

Please explain the changes you made here.

Checklist

  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the documentation, if necessary

Copy link

codecov bot commented Dec 22, 2024

Codecov Report

Attention: Patch coverage is 87.58621% with 36 lines in your changes missing coverage. Please review.

Project coverage is 81.06%. Comparing base (5f2433a) to head (cf74758).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
app/core/schools/endpoints_schools.py 81.81% 14 Missing ⚠️
app/core/users/endpoints_users.py 56.52% 10 Missing ⚠️
app/modules/cdr/endpoints_cdr.py 62.50% 3 Missing ⚠️
app/utils/initialization.py 75.00% 3 Missing ⚠️
app/app.py 84.61% 2 Missing ⚠️
app/modules/calendar/endpoints_calendar.py 50.00% 1 Missing ⚠️
app/modules/campaign/endpoints_campaign.py 85.71% 1 Missing ⚠️
app/modules/phonebook/endpoints_phonebook.py 75.00% 1 Missing ⚠️
app/modules/raffle/endpoints_raffle.py 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #641      +/-   ##
==========================================
- Coverage   81.31%   81.06%   -0.26%     
==========================================
  Files         129      134       +5     
  Lines       10013    10318     +305     
==========================================
+ Hits         8142     8364     +222     
- Misses       1871     1954      +83     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Daihecyy Daihecyy linked an issue Dec 23, 2024 that may be closed by this pull request
app/core/schools/cruds_schools.py Show resolved Hide resolved
app/core/models_core.py Outdated Show resolved Hide resolved
app/core/schools/cruds_schools.py Show resolved Hide resolved
app/core/schools/cruds_schools.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/users/endpoints_users.py Outdated Show resolved Hide resolved
migrations/versions/27-schools.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/schools/schools_type.py Show resolved Hide resolved
tests/test_schools.py Outdated Show resolved Hide resolved
app/core/groups/groups_type.py Show resolved Hide resolved
app/core/models_core.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Show resolved Hide resolved
app/core/models_core.py Outdated Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
@Marc-Andrieu Marc-Andrieu mentioned this pull request Dec 25, 2024
4 tasks
app/core/schools/endpoints_schools.py Dismissed Show dismissed Hide dismissed
app/core/models_core.py Outdated Show resolved Hide resolved
app/core/schemas_core.py Show resolved Hide resolved
app/core/schools/endpoints_schools.py Show resolved Hide resolved
app/core/users/endpoints_users.py Show resolved Hide resolved
app/core/schools/endpoints_schools.py Show resolved Hide resolved
app/core/schools/endpoints_schools.py Show resolved Hide resolved
app/core/schools/endpoints_schools.py Outdated Show resolved Hide resolved
app/core/users/cruds_users.py Outdated Show resolved Hide resolved
@armanddidierjean armanddidierjean added enhancement New feature or request core labels Jan 2, 2025
foucblg pushed a commit that referenced this pull request Jan 6, 2025
> [!IMPORTANT]
> We really need to release new versions **soon**, as we have not for
several months and there's some time needed to test in alpha before
prod.

This goes hand in hand with the **major version bump** [for
Titan](../../Titan/pull/462).

Both PRs are ready, and _seemingly_ waiting for some more **breaking
changes** to be merged, including (by likelihood):

- [ ] Allow students from other Schools to have a MyECL account: #641 
- [ ] MyECLPay: #611 
- [ ] "_Centrale Mega Meme_" Meme module: [no PR yet] [see
branch](../tree/cmm)
- [x] External Notifications: #613
db_school = models_core.CoreSchool(
id=school.value,
name=school.name,
email_regex=".*",
Copy link
Member

Choose a reason for hiding this comment

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

We should probably set a regex

initialization.create_school_sync(school=db_school, db=db)
except IntegrityError as error:
hyperion_error_logger.fatal(
f"Startup: Could not add group {db_school.name}<{db_school.id}> in the database: {error}",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f"Startup: Could not add group {db_school.name}<{db_school.id}> in the database: {error}",
f"Startup: Could not add school {db_school.name}<{db_school.id}> in the database: {error}",

Comment on lines +132 to +133
name: Mapped[str] = mapped_column(String, unique=True)
email_regex: Mapped[str] = mapped_column(String)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: Mapped[str] = mapped_column(String, unique=True)
email_regex: Mapped[str] = mapped_column(String)
name: Mapped[str] = mapped_column(unique=True)
email_regex: Mapped[str]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improving utils function name
3 participants