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 teachers to substitution plan #1647

Merged
merged 11 commits into from
Jun 9, 2024
8 changes: 3 additions & 5 deletions app/lib/dashboard/bloc/build_lesson_views.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,13 @@ LessonView _buildLessonView(
required Date date,
}) {
final timeline = _getTimeStatus(lesson.startTime, lesson.endTime);
final substitution = lesson.getSubstitutionFor(date);
final newLocation = substitution is LocationChangedSubstitution
? substitution.newLocation
: null;
final locationSubstitution =
lesson.getSubstitutionFor(date).getLocationChangedSubstitution();
return LessonView(
start: lesson.startTime.toString(),
end: lesson.endTime.toString(),
lesson: lesson,
room: newLocation ?? lesson.place,
room: locationSubstitution?.newLocation ?? lesson.place,
design: groupInfo?.design ?? Design.standard(),
abbreviation: groupInfo?.abbreviation ?? "",
timeStatus: timeline,
Expand Down
15 changes: 7 additions & 8 deletions app/lib/timetable/src/models/lesson.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
// SPDX-License-Identifier: EUPL-1.2

import 'package:cloud_firestore_helper/cloud_firestore_helper.dart';
import 'package:collection/collection.dart';
import 'package:date/date.dart';
import 'package:date/weekday.dart';
import 'package:date/weektype.dart';
Expand Down Expand Up @@ -132,14 +131,14 @@ class Lesson {
);
}

/// Returns the substitution for the given [date].
/// Returns the substitutions for the given [date].
///
/// If there is no substitution for the given [date], `null` will be returned.
Substitution? getSubstitutionFor(Date date) {
return substitutions.values.firstWhereOrNull(
(substitution) =>
substitution.date == date && substitution.isDeleted == false,
);
/// If there is no substitution for the given [date], the list will be empty.
List<Substitution?> getSubstitutionFor(Date date) {
return substitutions.values
.where((substitution) =>
substitution.date == date && substitution.isDeleted == false)
.toList();
}

@override
Expand Down
57 changes: 57 additions & 0 deletions app/lib/timetable/src/models/substitution.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// SPDX-License-Identifier: EUPL-1.2

import 'package:cloud_firestore/cloud_firestore.dart';
import 'package:collection/collection.dart';
import 'package:common_domain_models/common_domain_models.dart';
import 'package:date/date.dart';
import 'package:sharezone/timetable/src/models/substitution_id.dart';
Expand All @@ -18,13 +19,17 @@ enum SubstitutionType {
/// The lesson is moved to another room.
locationChanged,

/// The teacher changed for the lesson.
teacherChanged,

/// Unknown substitution type.
unknown;

String toDatabaseString() {
return switch (this) {
lessonCanceled => 'lessonCanceled',
locationChanged => 'placeChanged',
teacherChanged => 'teacherChanged',
unknown => 'unknown',
};
}
Expand All @@ -33,6 +38,7 @@ enum SubstitutionType {
return switch (value) {
'lessonCanceled' => lessonCanceled,
'placeChanged' => locationChanged,
'teacherChanged' => teacherChanged,
_ => unknown,
};
}
Expand Down Expand Up @@ -94,6 +100,14 @@ sealed class Substitution {
isDeleted: isDeleted,
updatedBy: updatedBy,
),
SubstitutionType.teacherChanged => TeacherChangedSubstitution(
id: id,
date: Date.parse(data['date'] as String),
createdBy: createdBy,
newTeacher: data['newTeacher'] as String,
Copy link
Member Author

Choose a reason for hiding this comment

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

Or should we use substitutionTeacher?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think substitutionTeacher is better.

isDeleted: isDeleted,
updatedBy: updatedBy,
),
SubstitutionType.unknown => UnknownSubstitution(
id: id,
date: Date.parse(data['date'] as String),
Expand Down Expand Up @@ -154,6 +168,29 @@ class LocationChangedSubstitution extends Substitution {
}
}

class TeacherChangedSubstitution extends Substitution {
final String newTeacher;

const TeacherChangedSubstitution({
required super.id,
required this.newTeacher,
required super.date,
required super.createdBy,
super.isDeleted,
super.updatedBy,
}) : super(type: SubstitutionType.teacherChanged);

@override
Map<String, dynamic> toCreateJson({
required bool notifyGroupMembers,
}) {
return {
...super.toCreateJson(notifyGroupMembers: notifyGroupMembers),
'newTeacher': newTeacher,
};
}
}

Comment on lines +182 to +193
Copy link
Collaborator

Choose a reason for hiding this comment

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

You did it like this so that only one class for a Substitution is needed both for creating a new substitution and reading an existing substitution out of a lesson, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean its the pattern you already used, but I can't remember if there was a specific reason why

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if I correctly understand you, do mean like splitting in into a DTO class or something like this? If yes: I used just one class to keep it simple for now

class UnknownSubstitution extends Substitution {
const UnknownSubstitution({
required super.id,
Expand All @@ -163,3 +200,23 @@ class UnknownSubstitution extends Substitution {
super.updatedBy,
}) : super(type: SubstitutionType.unknown);
}

extension SubstitutionList on List<Substitution?> {
LocationChangedSubstitution? getLocationChangedSubstitution() {
return firstWhereOrNull(
(substitution) => substitution is LocationChangedSubstitution)
as LocationChangedSubstitution?;
}

TeacherChangedSubstitution? getTeacherChangedSubstitution() {
return firstWhereOrNull(
(substitution) => substitution is TeacherChangedSubstitution)
as TeacherChangedSubstitution?;
}

LessonCanceledSubstitution? getLessonCanceledSubstitution() {
return firstWhereOrNull(
(substitution) => substitution is LessonCanceledSubstitution)
as LessonCanceledSubstitution?;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,29 @@ class SubstitutionController {
}));
}

void addTeacherSubstitution({
required String lessonId,
required bool notifyGroupMembers,
required Date date,
required String newTeacher,
}) {
final substitution = TeacherChangedSubstitution(
id: _generateId(),
date: date,
createdBy: userId,
newTeacher: newTeacher,
);
gateway.addSubstitutionToLesson(
lessonId: lessonId,
notifyGroupMembers: notifyGroupMembers,
substitution: substitution,
);
analytics
.log(NamedAnalyticsEvent(name: 'substitution_teacher_changed', data: {
'notify_group_members': notifyGroupMembers,
}));
}

void updatePlaceSubstitution({
required String lessonId,
required SubstitutionId substitutionId,
Expand All @@ -101,6 +124,24 @@ class SubstitutionController {
}));
}

void updateTeacherSubstitution({
required String lessonId,
required SubstitutionId substitutionId,
required bool notifyGroupMembers,
required String newTeacher,
}) {
gateway.updateSubstitutionInLesson(
lessonId: lessonId,
notifyGroupMembers: notifyGroupMembers,
newTeacher: newTeacher,
substitutionId: substitutionId,
);
analytics
.log(NamedAnalyticsEvent(name: 'substitution_teacher_changed', data: {
'notify_group_members': notifyGroupMembers,
}));
}

Future<String?> getMemberName(String courseId, UserId userId) async {
final member = await courseMemberAccessor.getMember(courseId, userId);
return member?.name;
Expand Down
Loading
Loading