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

Breaks in 24-25 year for reports #66

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
(datetime.date(2024, 1, 8), datetime.date(2024, 4, 28)),
(datetime.date(2024, 5, 20), datetime.date(2024, 7, 29)),
]

BREAKS = [
(datetime.date(2024, 11, 25), datetime.date(2024, 11, 29)),
(datetime.date(2024, 3, 10), datetime.date(2024, 3, 15)),
]

SCHWARTZ_EMAIL = "[email protected]"


Expand Down
41 changes: 19 additions & 22 deletions src/reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@

import discord
import gspread
import gspread_asyncio
from discord.ext import commands

from .constants import SCHWARTZ_EMAIL, Team, semester_given_date
from .constants import BREAKS, SCHWARTZ_EMAIL, Team, semester_given_date
from .email import Email
from .tasks import run_on_weekday
from .utils import is_active, ordinal
Expand Down Expand Up @@ -62,8 +61,18 @@ def _end_date(cls) -> datetime.date:
raise RuntimeError("No semester is occurring right now!")
return semester[1]

@staticmethod
def _is_break(date: datetime.date) -> bool:
return any(start <= date <= end for start, end in BREAKS)

def _date_to_index(self, date: datetime.date) -> int:
return (date - self._start_date()).days // 7 + 1
current_date = self._start_date()
week_index = 0
while current_date <= date:
if not self._is_break(current_date):
week_index += 1
current_date += datetime.timedelta(days=7)
return week_index

@property
def week(self) -> int:
Expand All @@ -78,7 +87,12 @@ def date_range(self) -> tuple[datetime.date, datetime.date]:
"""
Inclusive date range for this column.
"""
start_date = self._start_date() + datetime.timedelta(weeks=self.week)
start_date = self._start_date()
current_week = 0
while current_week < self.week:
if not self._is_break(start_date):
current_week += 1
start_date += datetime.timedelta(days=7)
Comment on lines +90 to +95
Copy link
Member

Choose a reason for hiding this comment

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

I think this approach is a good start, but might have a bit of an issue. It appears this implementation completely ignores break weeks (ie, it pretends break weeks don't exist). This isn't really what we want, as we want people to be able to submit reports during break weeks, but we just don't want to make them required.

You probably don't even need to change anything in the calculation of weeks. You just need to change the behavior of the reports system:

  1. The reports submission button should display "Submit your report! (optional due to break)" (relevant code)
  2. Only the first individual reminder should go off, so the other two will need to check whether we are currently in a break week and then disable their behavior.
  3. Rather than sending out the grading form to leaders, everyone should be automatically graded green. (relevant function)

end_date = start_date + datetime.timedelta(days=6)
return start_date, end_date

Expand All @@ -91,7 +105,7 @@ def closes_at(self) -> datetime.datetime:

@classmethod
def from_date(cls, date: datetime.date):
col_offset = (date - cls._start_date()).days // 7
col_offset = cls._date_to_index(cls, date)
# Each week has two columns: one for the report and one for the score
# +1 because columns are 1-indexed
return cls((col_offset * 2) + 1 + len(Column))
Expand Down Expand Up @@ -711,23 +725,6 @@ def __init__(self, bot: MILBot):
self.second_individual_reminder.start(self)
self.update_report_channel.start(self)

@run_on_weekday(calendar.FRIDAY, 12, 0, check=is_active)
async def post_reminder(self):
general_channel = self.bot.general_channel
return await general_channel.send(
f"{self.bot.egn4912_role.mention}\nHey everyone! Friendly reminder to submit your weekly progress reports by **Sunday night at 11:59pm**. You can submit your reports in the {self.bot.member_services_channel.mention} channel. If you have any questions, please contact your leader. Thank you!",
)

async def safe_col_values(
self,
ws: gspread_asyncio.AsyncioGspreadWorksheet,
column: int,
) -> list[str]:
names = await ws.col_values(column)
if not isinstance(names, list):
raise RuntimeError("Column is missing!")
return [n or "" for n in names]

Comment on lines -714 to -730
Copy link
Member

Choose a reason for hiding this comment

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

why delete these? :(

async def students_status(self, column: int) -> list[Student]:
main_worksheet = await self.bot.sh.get_worksheet(0)
names = await self.safe_col_values(main_worksheet, Column.NAME_COLUMN)
Expand Down
Loading