-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
… and fulfilledrequests
…e views related to those two
There was a problem hiding this 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.
There was a problem hiding this 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.
<template> | ||
<div class="container"> | ||
<h3>Select a Date Range</h3> | ||
<VueDatePicker |
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
…eaders from a lot of files. more pr fixes
There was a problem hiding this 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?
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:
sessionsStore
and other small changes.UpcomingBookings
andMyGallery
are updated to reflect changes of the storeAlright, now to the PR
STORE CHANGES
sessions store
state
no longer hassessions
. Now it hasfulfilledRequests
,upcomingRealTimeSessions
, andrequestedObservations
.fulfilledRequests
: these areNORMAL
observations that have the state ofCOMPLETED
andREAL_TIME
observations that have an end value that's in the past (16 minutes ago or more) since their state is alwaysPENDING
. These values are sorted in the actionsortSessions
which is a success callback offetchSessions
. This array is used to obtain the observation id and make a request to the archive to obtain the thumbnails and display them onMyGallery
upcomingRealTimeSessions
: these areREAL_TIME
observations that are in the future and are also sorted insortSessions
requestedObservations
: these areNORMAL
observations with the state ofPENDING
. 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 aNORMAL
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 theobservation_id
to render the images related to the observation id.fetchSessions
: fetches sessions by making a request to the/observations
endpoint. The success callback issortSessions
sortSessions
: this callback sorts sessions based on the following criteria: if the state isCOMPLETE
or the observation type isREAL_TIME
and the end is 16 minutes ago or more, then the response gets stored infulfilledRequests
fetchPendingObservations
: this stores all the results inrequestedObservations
. It makes a request to the/requestgroups
endpoint. This is used to display thePending Observations
(seen below)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 componentSchedulingSettings
renders. This component will not render the project name, target, ra, dec fields that you see inAdvancedScheduling
because the props passed to theSchedulingSettings
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 foroperator
isSINGLE
. And if there is more than one target, the value isMANY
. 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 windowscheduleObservation
: gathers request data, constructs the request list for single or multiple targets and sends a POST request to the/requestgroup
endpointUPDATED FIXES:
Renamed consts, changed some hardcoded values, added more comments, added magic numbers, but the biggest change was splitting the
sessions
store (now calledrealTimeSessions
). There are now two stores:realTimeSessions
andobsPortalData
. 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 actionfetchCompleteObservationsAndUpcomingRTSessions
sorts these two since it's the same api request.