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

Fixes suggested by Jordan / Luke #650

Merged
merged 14 commits into from
Apr 10, 2024
6 changes: 4 additions & 2 deletions backend/courses/management/commands/recompute_soft_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ def recompute_enrollment():
)


# course credits = sum(section credis for all activities)
# course credits = sum(section credis for all activities for sections below 500)
# the < 500 heuristic comes from here:
# https://provider.www.upenn.edu/computing/da/dw/student/enrollment_section_type.e.html
COURSE_CREDITS_RAW_SQL = dedent(
"""
WITH CourseCredits AS (
Expand All @@ -108,6 +110,7 @@ def recompute_enrollment():
INNER JOIN (
SELECT MAX(U1."credits") AS "activity_cus", U1."course_id"
FROM "courses_section" U1
WHERE U1."code" < '500' AND (U1."status" <> 'X' OR U1."status" <> '')
GROUP BY U1."course_id", U1."activity"
) AS U2
ON U0."id" = U2."course_id"
Expand All @@ -125,7 +128,6 @@ def recompute_enrollment():
def recompute_course_credits(
model=Course, # so this function can be used in migrations (see django.db.migrations.RunPython)
):

with connection.cursor() as cursor:
cursor.execute(COURSE_CREDITS_RAW_SQL)

Expand Down
8 changes: 6 additions & 2 deletions backend/courses/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,12 @@ def filter_by_semester(self, queryset):
semester = self.get_semester()
if semester != "all":
queryset = queryset.filter(**{self.get_semester_field(): semester})
else:
queryset = queryset.order_by("full_code", "-semester").distinct("full_code")
else: # Only used for Penn Degree Plan (as of 4/10/2024)
queryset = (
queryset.exclude(credits=None) # heuristic: if the credits are empty, then ignore
.order_by("full_code", "-semester")
.distinct("full_code")
)
return queryset

def get_queryset(self):
Expand Down
20 changes: 12 additions & 8 deletions backend/degree/serializers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from textwrap import dedent

from django.db.models import Q
from django.db.models import Q, Subquery
from rest_framework import serializers

from courses.models import Course
Expand Down Expand Up @@ -143,13 +143,17 @@ def validate(self, data):
for rule in rules:
# NOTE: we don't do any validation if the course doesn't exist in DB. In future,
# it may be better to prompt user for manual override
if (
Course.objects.filter(full_code=full_code).exists()
and not Course.objects.filter(rule.get_q_object(), full_code=full_code).exists()
):
raise serializers.ValidationError(
f"Course {full_code} does not satisfy rule {rule.id}"
)
if Course.objects.filter(full_code=full_code).exists():
satisfying_courses = Course.objects.filter(rule.get_q_object())
if not (
Course.objects.filter(
full_code=full_code,
topic_id__in=Subquery(satisfying_courses.values("topic_id")),
).exists()
):
raise serializers.ValidationError(
f"Course {full_code} does not satisfy rule {rule.id}"
)

# Check for double count restrictions
double_count_restrictions = DoubleCountRestriction.objects.filter(
Expand Down
13 changes: 13 additions & 0 deletions backend/tests/courses/test_recompute_soft_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ def setUp(self):
"CIS-1210-001", TEST_SEMESTER
)

# Implictly testing that we exclude sections with code > 500
_, self.section5, _, _ = get_or_create_course_and_section("CIS-1210-500", TEST_SEMESTER)
self.section5.credits = 10.0

def test_null_section_credits(self):
self.assertIsNone(self.course3.credits)
self.assertIsNone(self.section4.credits)
Expand Down Expand Up @@ -267,3 +271,12 @@ def test_same_activity_null_credits(self):
recompute_course_credits()
self.course.refresh_from_db()
self.assertEqual(self.course.credits, 2.00)

def test_excludes_sections_with_status_besides_closed_and_open(self):
_, cancelled_section, _, _ = get_or_create_course_and_section("CIS-160-102", TEST_SEMESTER)
cancelled_section.credits = 10.0
cancelled_section.status = "X"
recompute_course_credits()

self.course2.refresh_from_db()
self.assertEqual(self.course2.credits, 1.50)
2 changes: 2 additions & 0 deletions backend/tests/degree/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
SatisfactionStatus,
)
from degree.serializers import SimpleCourseSerializer
from tests.courses.util import fill_course_soft_state


TEST_SEMESTER = "2023C"
Expand Down Expand Up @@ -98,6 +99,7 @@ def setUp(self):
self.cis_1930, self.cis_1930_001, _, _ = get_or_create_course_and_section(
"CIS-1920-001", TEST_SEMESTER, course_defaults={"credits": 1}
)
fill_course_soft_state()

self.degree = Degree.objects.create(program="EU_BSE", degree="BSE", major="CIS", year=2023)
self.parent_rule = Rule.objects.create()
Expand Down
14 changes: 9 additions & 5 deletions frontend/degree-plan/components/Dock/Dock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,14 @@ const DockContainer = styled.div<{$isDroppable:boolean, $isOver: boolean}>`
const SearchIconContainer = styled.div`
padding: .25rem 2rem;
padding-left: 0;
border-color: var(--primary-color-extra-dark);
border-color: var(--primary-color-xx-dark);
color: var(--primary-color-extra-dark);
border-width: 0;
border-right-width: 2px;
border-style: solid;
flex-shrink: 0;
display: flex;
gap: 1rem;
`

const DockedCoursesWrapper = styled.div`
Expand Down Expand Up @@ -106,7 +109,7 @@ const Dock = ({ user, login, logout, activeDegreeplanId }: DockProps) => {
// const [courseAdded, setCourseAdded] = React.useState(false);
const { searchPanelOpen, setSearchPanelOpen, setSearchRuleQuery, setSearchRuleId } = useContext(SearchPanelContext)
const { createOrUpdate } = useSWRCrud<DockedCourse>(`/api/degree/docked`, { idKey: 'full_code' });
const {data: dockedCourses = [], isLoading} = useSWR<DockedCourse[]>(user ? `/api/degree/docked` : null);
const { data: dockedCourses = [], isLoading } = useSWR<DockedCourse[]>(user ? `/api/degree/docked` : null);

// Returns a boolean that indiates whether this is the first render
const useIsMount = () => {
Expand All @@ -117,8 +120,6 @@ const Dock = ({ user, login, logout, activeDegreeplanId }: DockProps) => {
return isMountRef.current;
};

const isMount = useIsMount();

const [{ isOver, canDrop }, drop] = useDrop(() => ({
accept: [ItemTypes.COURSE_IN_PLAN, ItemTypes.COURSE_IN_REQ],
drop: (course: DnDCourse) => {
Expand Down Expand Up @@ -160,8 +161,11 @@ const Dock = ({ user, login, logout, activeDegreeplanId }: DockProps) => {
setSearchPanelOpen(!searchPanelOpen);
}}>
<DarkBlueIcon>
<i className="fas fa-search fa-lg"/>
<i className="fas fa-plus fa-lg"/>
</DarkBlueIcon>
<div>
Add Course
</div>
</SearchIconContainer>
<DockedCoursesWrapper>
{isLoading ?
Expand Down
8 changes: 3 additions & 5 deletions frontend/degree-plan/components/Footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@ const Footer = () => (
Penn Labs
</Link>
.
Have feedback about Penn Degree Plan? Let us know: {" "}
<Link href="mailto:[email protected]">[email protected]</Link>
{
// TODO: uncomment once out of beta
// <Link href="https://airtable.com/appFRa4NQvNMEbWsA/shrzXeuiEFF8OD89P">here!</Link>
Have feedback about Penn Degree Plan? Let us know {" "}
{// <Link href="mailto:[email protected]">[email protected]</Link>
}
<Link href="https://airtable.com/appFRa4NQvNMEbWsA/shr120VUScuNJywyv">here!</Link>
</Wrapper>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ const ScheduleDropdownHeader = styled.div`
`

const SelectedName = styled.span`
font-weight: 500;
font-weight: 700;
min-width: 5rem;
font-size: 1.25rem;
`
Expand Down
49 changes: 29 additions & 20 deletions frontend/degree-plan/components/FourYearPlan/Semesters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,22 +66,41 @@ const AddButton = styled.div`
gap: 1rem;
`;

const YearInput = styled.input`
width: 9rem;
background-color: transparent;
border-color: #9FB5EF;
color: #C1C1C1;
box-shadow: none;
&:hover {
borderColor: "#9FB5EF";
}

padding: .75rem;
padding-top: .5rem;
padding-bottom: .5rem;
border-style: solid;
border-radius: .25rem;
border-width: 1px;
border-top-left-radius: 0;
border-top-right-radius: 0;
font-size: 1rem;
`

const selectStyles = (topOrBottom: boolean) => ({
control: (provided: any) => ({
...provided,
width: "130px",
width: "9rem",
backgroundColor: "transparent",
borderColor: "#9FB5EF",
color: "#C1C1C1",
boxShadow: "none",
"&:hover": {
borderColor: "#9FB5EF",
},
...(
topOrBottom ?
{ borderBottomLeftRadius: 0, borderBottomRightRadius: 0, borderBottom: 0 } :
{ borderTopLeftRadius: 0, borderTopRightRadius: 0 }
)
borderBottomLeftRadius: 0,
borderBottomRightRadius: 0,
borderBottom: 0
}),
singleValue: (provided: any) => ({
...provided,
Expand Down Expand Up @@ -126,14 +145,6 @@ const ModifySemesters = ({
{ value: "C", label: "Fall" },
];

// TODO: Un-hardcode years
const yearOptions = [
{ value: "2024", label: "2024" },
{ value: "2025", label: "2025" },
{ value: "2026", label: "2026" },
{ value: "2027", label: "2027" },
];

return (
// TODO: add a modal for this
<AddSemesterContainer className={className}>
Expand All @@ -153,11 +164,10 @@ const ModifySemesters = ({
onChange={(option) => setSelectedSeason(option ? option.value : selectedSeason)}
/>

<Select
styles={selectStyles(false)}
options={yearOptions}
value={yearOptions.find((option) => option.value === selectedYear)}
onChange={(option) => setSelectedYear(option ? option.value : selectedYear)}
<YearInput
value={selectedYear}
type="number"
onChange={(e) => setSelectedYear(e.target.value)}
/>
</AddSemesterContainer>
);
Expand Down Expand Up @@ -247,7 +257,6 @@ const Semesters = ({
useEffect(() => {
if (Object.keys(semesters).length == 0 && !isLoading) setEditMode(true);
// if finish loading and no semesters, we go to edit mode for the user to add new semesters
else setEditMode(false);
if (!activeDegreeplan) return;
if (typeof window !== "undefined" && Object.keys(semesters).length) {
localStorage.setItem(
Expand Down
4 changes: 2 additions & 2 deletions frontend/degree-plan/components/Requirements/ReqPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const DegreeHeaderContainer = styled.div`

const ReqPanelTitle = styled.div`
font-size: 1.25rem;
font-weight: 500;
font-weight: 700;
`

const DegreeBody = styled.div`
Expand Down Expand Up @@ -82,7 +82,7 @@ const AddButton = styled.div`

const ReqPanelBody = styled(PanelBody)`
overflow-y: scroll;
padding: .75rem;
padding: 1.5rem;
`

interface DegreeHeaderProps {
Expand Down
2 changes: 1 addition & 1 deletion frontend/degree-plan/components/Search/SearchPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const PanelContainer = styled.div`
`;

const PanelTitle = styled.div`
font-weight: 500;
font-weight: 700;
`

const SearchPanelHeader = styled(PanelHeader)`
Expand Down
2 changes: 2 additions & 0 deletions frontend/degree-plan/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
--callout-border-rgb: 172, 175, 176;
--card-rgb: 180, 185, 188;
--card-border-rgb: 131, 134, 135;

font-size: 14px;
}

/*
Expand Down
Loading