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

Delete schedule #108

Open
wants to merge 260 commits into
base: master
Choose a base branch
from
Open

Delete schedule #108

wants to merge 260 commits into from

Conversation

todesj
Copy link
Collaborator

@todesj todesj commented Nov 8, 2021

No description provided.

@todesj todesj requested a review from BraydenKO as a code owner November 8, 2021 00:40
Copy link
Member

@Levi-Lesches Levi-Lesches left a comment

Choose a reason for hiding this comment

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

Looking good, just you need to use the database the way RamLife sets it up. Similar to using models, you don't just do User.name, you look up Models.instance.user.name, you're going to have to do the same type of thing with the database. Good news is you have plenty of examples.

Also, never import 1) a src folder, or 2) deep into a "layer" (anything deeper than lib). Either refer to the file by name if they're next to each other, or modify the lib/layer.dart to include that file

@@ -73,6 +74,8 @@ class ScheduleModel extends Model {
day == null ? null : Day.fromJson(day)
]
];
Schedule.defaults = Map.from(await CloudCalendar.schedules.
Copy link
Member

Choose a reason for hiding this comment

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

You can't use the actual class Cloud Calendar here, you need to use an instance (look up static vs instance methods if you're confused). Get the database from:

Services.instance.database.schedule

And call default schedules on that. Of course, you'll need to implement that too, so look for how the database downloads the schedules and try to copy that.

Comment on lines 6 to 7
import 'package:ramaz/src/services/databases/calendar/implementation.dart';
import 'package:ramaz/src/services/firestore.dart';
Copy link
Member

Choose a reason for hiding this comment

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

Delete these, see below

Comment on lines +131 to +138
/// This function rep
// Future<void> deleteSchedules(Map<DateTime,Day> days) async {
// for (MapEntry<DateTime, Day> entry in days.entries){
// entry.value.schedule!=ScheduleViewModel.defatulSchedule[entry.value.name];
// await updateDay(day: entry.value, date: entry.key);
// }
// }
Copy link
Member

Choose a reason for hiding this comment

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

Delete this I guess, and the import above

lib/src/models/data/schedule.dart Outdated Show resolved Hide resolved
lib/src/models/data/schedule.dart Outdated Show resolved Hide resolved


/// A Map of weekdays to their default schedule
static late Map<String, Schedule> defaults;
Copy link
Member

Choose a reason for hiding this comment

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

You can do this in three ways:

  1. static late, which means there is no value until it's set
  2. Map<String, Schedule>?, which means it's null until set
  3. defaults = {};, which means it's empty until set.

While 3 is easier to work with, I think you should do 1 or 2, since it implies it's invalid to search up a default schedule before the data is loaded. You can follow my example and use late, which implies there should always be some data and not having data is an error, but I think you should use a nullable map since late is kinda hacky in Dart but null is much more "normal".

lib/src/models/data/schedule.dart Outdated Show resolved Hide resolved
lib/src/models/view/calendar_editor.dart Outdated Show resolved Hide resolved
@@ -34,12 +34,13 @@ class CloudCalendar implements CalendarInterface {
Future<List<Map>> getSchedules() async => List<Map>.from(
(await schedules.throwIfNull("Cannot find schedules")) ["schedules"]
);

@override
Future<Map<String, String>> getDefaultSchedules() async => Map.from(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Future<Map<String, String>> getDefaultSchedules() async => Map.from(
Future<Map<String, String>> getDefaultSchedules() async =>

You don't need Map here, unless you need to cast to a Map<String, String> specifically (which you might).

@@ -68,6 +69,9 @@ class LocalCalendar implements CalendarInterface {
.getAll(Idb.scheduleStoreName);

@override
Future<Map<String, String>> getDefaultSchedules() => Idb.instance
.get("schedules","Monday") as Future<Map<String, String>>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.get("schedules","Monday") as Future<Map<String, String>>;
.get("schedules","Monday");
  1. No as, see above
  2. Why Monday?

lib/src/services/databases/calendar/interface.dart Outdated Show resolved Hide resolved
@@ -116,6 +116,9 @@ class Idb extends DatabaseService {
/// The name for the schedules object store.
static const String scheduleStoreName = "schedules";

/// The name for the default schedules object store.
static const String defaultScheduleStoreName = "defaults";
Copy link
Member

Choose a reason for hiding this comment

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

Ah, okay here's something -- should this really be a separate data store? Think of it like a "table" in SQL or a "collection" in Firestore: can you really say this is a whole collection of similarly-formatted entries? I think this should be filed under "miscellaneous" data somehow -- maybe use the SharedPreferences service? Sure, it's a bit weird in that you're using one service from another but I think RamLife can handle it.

@@ -141,7 +145,8 @@ class Idb extends DatabaseService {
..createObjectStore(calendarStoreName, keyPath: "month")
..createObjectStore(reminderStoreName, keyPath: "id")
..createObjectStore(sportsStoreName, autoIncrement: true)
..createObjectStore(scheduleStoreName, keyPath: "name");
..createObjectStore(scheduleStoreName, keyPath: "name")
..createObjectStore(defaultScheduleStoreName, keyPath: "Monday");
Copy link
Member

Choose a reason for hiding this comment

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

See above, I don't think this is the way to go about it.

@CLAassistant
Copy link

CLAassistant commented Nov 24, 2021

CLA assistant check
All committers have signed the CLA.

todesj and others added 17 commits January 8, 2022 18:12
* Added schedule_search.dart

* Added schedule search model

* Update lib/src/models/view/schedule_search.dart

Co-authored-by: Levi Lesches <[email protected]>

* Update lib/src/models/view/schedule_search.dart

Co-authored-by: Levi Lesches <[email protected]>

* Fixes some formatting

* Added docs and exported schedule_search.dart in lib/models.dart

* Cleaned up schedule_search.dart

* Cleaned up models.dart

* Added Subject.id, PeriodData.name, PerioData.dayName

* Update lib/src/data/schedule/subject.dart

Co-authored-by: Levi Lesches <[email protected]>

* Add Period.dayName, and Period.name (Student-Lyf#81)

* Converted firestore/lib/src/helpers to firestore-py/lib/utils

* Ported firestore/lib/src/services to firestore-py/lib/services

* Removed (and revoked) Firebase credentials

* Cleanup

* Cleaned up logging/Firebase issues

* Added some __init__.py

* Ported firebase/firestore/node/calendar.dart to firebase/firestore-py/bin/calendar.py

* Ported firebase/firestore/node/admins.dart to firebase/firestore-py/bin/admins.py

* Ported firebase/firestore/node/feedback.dart to firebase/firestore-py/bin/feedback.py

* Added some data python files and sections python files

* Continued translating more files (Note: some are incomplete!)
lib/data/schedule
lib/sections/reader
edited lib/utils/dir to use constants from constants.yaml

* Added bin/faculty, bin/secions, facultylogic and facultyreader python files
Also added constants.py to get data from constants.yaml

* Ported firebase/firestore/node/students.dart --> firebase/firestore-py/bin/students.py

* Cleanup

* Delete student.py

* Fix students.py and uploading

* Added some more python files. many variable should be converted to
snake_case

* Changed camel case variables to snake_case

* small edits, opted for a different constants.py file too

* Fixed some typos

* Fixed bug where testers had no schedule (instead of a blank one)

* ---

* Finished scripts to upload data

* Changed dayNames to a list rather than set

* Make Fridays a Friday schedule by default

* Modified feedback.py to actually output feedback

* Added students reader

* Cleanup

* Fixed a bug in faculty/logic.py where a (Student-Lyf#101)

teacher's periods weren't being added to theit schedule, instead the
teacher's schedule was being set to contain only one period.

Co-authored-by: todesj <[email protected]>
Co-authored-by: todesj <[email protected]>
Co-authored-by: Brayden Kohler <[email protected]>
Co-authored-by: BraydenKO <[email protected]>
* Enabled Emulator use

* Fixed emulator error

* Update lib/src/services/firebase_core.dart

Added doc comment for `FirebaseCore.shouldUseEmulator`

Co-authored-by: Levi Lesches <[email protected]>
Makes the schedule icon appear in the dashboard when the side sheet is not persistently open
Adds Podfile.lock to the repo and updates XCode.
…-Lyf#123)

* Moved Firebase initialization logic to main using flutterfire_cli

* Removed manual Firebase initialization for Web

* Moved Dart-only Firebase initialization to FirebaseCore.initializeFirebase

* Updated Firebase dependencies to support flutterfire_cli

* Removed old (manual) calls to FirebaseCore.init(), replaced with new flutterfire_cli initialization

* Added try/catch for Firestore emulator
  Workaround for firebase/flutterfire#6216

* Cleanup
@Levi-Lesches
Copy link
Member

I force-pushed to clean the history, now there are no merge conflicts

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.

4 participants