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

Add Team Assessments #968

Merged
merged 159 commits into from
Mar 26, 2024
Merged

Conversation

CheongYeeMing
Copy link
Contributor

@CheongYeeMing CheongYeeMing commented Jul 10, 2023

Source Academy - Team Assessments Module

This is the PR for the MVP of integrating Team Assessments into the existing Source Academy. Collaboration among students in coursework through the implementation of this collaborative module offers immense benefits and serves as a powerful motivation for its adoption. By providing students with the option to collaborate, share ideas, and work together on assessments, projects, and discussions, they can tap into a diverse range of perspectives and insights.

To be completed:

  • Migrations to modify and build onto existing schema
  • Model and Controller implementation for Teams
  • Configure router
  • Retrieve Team Assessments
  • Retrieve Team Submissions
  • Integrate into Notifications (Elixir)
  • Refactor SQL chunk in Assessments.exs
  • Retrieve Team Submissions for Grading
  • Retrieve Team Submission for Students
  • Allow saving of Team Submission
  • Implement the Safe-Save feature
  • Integrate into Grading System
  • Swagger Documentation
  • Add Teams to seeds.exs
  • Tests

Notes:

mix.lock Outdated Show resolved Hide resolved
@YaleChen299 YaleChen299 self-assigned this Jul 25, 2023
@RichDom2185 RichDom2185 requested a review from sayomaki January 23, 2024 12:16
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

Just one comment for now (reviewed ~ half the files):

Comment on lines +47 to +49
team_members
|> Enum.map(fn team_member ->
users |> Enum.find(&(&1.id == team_member.student_id))
Copy link
Member

Choose a reason for hiding this comment

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

I just realised this results in an n+1 query problem. We should use a join instead to keep it to O(1) total queries.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, on second thought, should be fine as there are no team members for now, but we might want to look at it again in the future. Will file an issue.

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot!

@RichDom2185 RichDom2185 merged commit 5bf61d9 into source-academy:master Mar 26, 2024
2 checks passed
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.

5 participants