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

Breaks in 24-25 year for reports #66

wants to merge 2 commits into from

Conversation

arjuntalati
Copy link

No description provided.

@arjuntalati arjuntalati requested a review from cbrxyz July 28, 2024 23:10
Copy link
Member

@cbrxyz cbrxyz left a comment

Choose a reason for hiding this comment

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

Great start! Thank you!

Comment on lines -714 to -730
@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]

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? :(

Comment on lines +90 to +95
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)
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)

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

Successfully merging this pull request may close these issues.

2 participants