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

Fix/schedule flow #72

Merged
merged 30 commits into from
Nov 1, 2024
Merged

Fix/schedule flow #72

merged 30 commits into from
Nov 1, 2024

Conversation

capetillo
Copy link
Collaborator

@capetillo capetillo commented Oct 25, 2024

FIX: SCHEDULE FLOW

First, thank you for your patience. It has been a very tumultuous time and lots of bugs to debug. Second, here's my PR.

NOTE ON NUMBER OF FILES CHANGED:

  • 6 files are tests; 4 new and 2 modified to reflect the changes of the sessionsStore and other small changes.
  • 1 file is deleted
  • UpcomingBookings and MyGallery are updated to reflect changes of the store

Alright, now to the PR

STORE CHANGES
sessions store

  • The state no longer has sessions. Now it has fulfilledRequests, upcomingRealTimeSessions, and requestedObservations.
  • fulfilledRequests: these are NORMAL observations that have the state of COMPLETED and REAL_TIME observations that have an end value that's in the past (16 minutes ago or more) since their state is always PENDING. These values are sorted in the action sortSessions which is a success callback of fetchSessions. This array is used to obtain the observation id and make a request to the archive to obtain the thumbnails and display them on MyGallery
  • upcomingRealTimeSessions: these are REAL_TIME observations that are in the future and are also sorted in sortSessions
  • requestedObservations: these are NORMAL observations with the state of PENDING. The difference between how this value is obtain vs the other two above is that the former hits the /requestgroups endpoint and the other two are obtained from the /observations endpoint. The reason for this is that, for one, when scheduling a NORMAL observation, the user makes a request to the /requestgroups endpoint because they may make more than one request per scheduling. And for all the other requests, hitting the /observations endpoint is necessary to obtain the observation_id to render the images related to the observation id.
  • fetchSessions: fetches sessions by making a request to the /observations endpoint. The success callback is sortSessions
  • sortSessions: this callback sorts sessions based on the following criteria: if the state is COMPLETE or the observation type is REAL_TIME and the end is 16 minutes ago or more, then the response gets stored in fulfilledRequests
  • fetchPendingObservations: this stores all the results in requestedObservations. It makes a request to the /requestgroups endpoint. This is used to display the Pending Observations (seen below)
Screenshot 2024-10-23 at 5 29 17 PM

SCHEDULING CHANGES
First, here's a video to better demonstrate both of the flows

Beginner scheduling view

Screen.Recording.2024-10-24.at.4.00.33.PM.mov

In the beginner view, the user starts first by selecting a date range. This date range is used to make an api request to whats up in the sky to get a list of all available targets (in N and S hemispheres) as well as recommended exposures associated with each target. The targets are sorted based on the object it corresponds to (e.g. M100 would go under galaxy) and only the first 3 targets are sorted and displayed. I can change this, but for now, we're only showing 3. The user then selects the desired target and if I'm ok with defaults is selected, then the selected target along with the recommended exposure settings are what's requested to the /requestgroups endpoint.
Alternatively, the user can select Let me choose and the component SchedulingSettings renders. This component will not render the project name, target, ra, dec fields that you see in AdvancedScheduling because the props passed to the SchedulingSettings component.

Advanced scheduling view

Screen.Recording.2024-10-24.at.4.02.50.PM.mov

Now in the advanced view, the SchedulingSettings renders entirely. The difference here is that fields are disabled and they become enabled as the user fills them out. For target, the ra and dec get auto populated by making a request to simbad2k. Then once a valid target is selected, the rest of the fields are enabled.

Both components (Advanced and Beginner) pass the selections through emits to the parent component, Scheduling View. It is here that the POST request to /requestgroups happens. If there is only one target, the value for operator is SINGLE. And if there is more than one target, the value is MANY. The requests get populated based on how many targets there are and each target is its own request (plus configuration)
In more detail:

  • createInstrumentConfigs: generates instrument configurations for each exposure (including exposure settings)
  • createRequest: constructs a request with target details, exposure settings, and time window
  • scheduleObservation: gathers request data, constructs the request list for single or multiple targets and sends a POST request to the /requestgroup endpoint

UPDATED FIXES:

Renamed consts, changed some hardcoded values, added more comments, added magic numbers, but the biggest change was splitting the sessions store (now called realTimeSessions). There are now two stores: realTimeSessions and obsPortalData. The former handles tokens for rt sessions, the current session's id, current session. The latter handles any completed observations (to display thumbnails), upcoming real time sessions, and pending request groups. The reason why upcoming real time sessions is handled here is because the action fetchCompleteObservationsAndUpcomingRTSessions sorts these two since it's the same api request.

@capetillo capetillo requested review from mgdaily, jnation3406 and zemogle and removed request for mgdaily and jnation3406 October 25, 2024 02:53
@zemogle zemogle mentioned this pull request Oct 28, 2024
12 tasks
Copy link

@jnation3406 jnation3406 left a comment

Choose a reason for hiding this comment

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

I didn't look at the tests since I don't know much about javascript testing - Matt can take a closer look at that. Just some comments on renaming things and being clearer on names matching up with what the data is in our language, i.e. requests vs observations. Also some places you might want to consider using an object/map to store lists of requests / observations keyed on their id, rather than having them as plain lists and filtering to get the subset you want.

src/stores/sessions.js Outdated Show resolved Hide resolved
src/stores/sessions.js Outdated Show resolved Hide resolved
src/stores/sessions.js Outdated Show resolved Hide resolved
src/stores/sessions.js Outdated Show resolved Hide resolved
src/stores/sessions.js Outdated Show resolved Hide resolved
src/components/Views/SchedulingView.vue Show resolved Hide resolved
src/components/Views/SchedulingView.vue Show resolved Hide resolved
src/components/Views/SchedulingView.vue Show resolved Hide resolved
src/components/Views/SchedulingView.vue Outdated Show resolved Hide resolved
src/components/Views/SchedulingView.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@zemogle zemogle left a comment

Choose a reason for hiding this comment

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

There is a lot of good stuff here. I've picked up some obvious things but someone else should check them. I did notice during my test that I don't get any thumbnails returned. I can't see an obvious reason for this. The requests to the thumbnail service are being sent. This will be a lot more responsive once the BANZAI at site pipeline puts thumbnails directly into the archive.

src/components/Scheduling/BeginnerScheduling.vue Outdated Show resolved Hide resolved
<template>
<div class="container">
<h3>Select a Date Range</h3>
<VueDatePicker
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a different date picker to the one in Live Observing. We should probably use the same in both for a consistent style.

Copy link
Contributor

@mgdaily mgdaily left a comment

Choose a reason for hiding this comment

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

Getting closer! I really like the functionality you've built, and my main concern now is code maintainability by you or anyone else who needs to touch this project.

Thanks for getting the PR out, and I appreciate the detailed writeup. Realistically, this PR is far too large for anyone to truly wrap their head around and meaningfully review via GitHub. I think it would help you and the project stay on track to map out small, isolated features that can be reviewed easily - let's make that a rule going forward. Please communicate with us when you need help breaking these development efforts up into smaller chunks - we are here to help.

src/components/Dashboard/UpcomingBookings.vue Outdated Show resolved Hide resolved
src/stores/sessions.js Outdated Show resolved Hide resolved
src/stores/sessions.js Outdated Show resolved Hide resolved
src/components/Views/SchedulingView.vue Outdated Show resolved Hide resolved
src/components/Views/SchedulingView.vue Outdated Show resolved Hide resolved
src/components/Views/SchedulingView.vue Show resolved Hide resolved
src/components/Scheduling/SchedulingSettings.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@mgdaily mgdaily left a comment

Choose a reason for hiding this comment

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

Mostly looks good, I just have a few comments of things needing to be addressed.

Can we capture all of the TODOs somewhere visible (Github project board?) so we can start picking away at them and pulling them off as small individual branches?

src/stores/realTimeSessions.js Outdated Show resolved Hide resolved
src/stores/obsPortalData.js Show resolved Hide resolved
src/stores/obsPortalData.js Show resolved Hide resolved
@capetillo capetillo merged commit 2eac46b into main Nov 1, 2024
1 check passed
@capetillo capetillo deleted the fix/schedule-flow branch November 1, 2024 20:34
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