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

Added some comments to the code #11

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

BenM-BenM
Copy link

No description provided.

Copy link
Member

@gabydd gabydd left a comment

Choose a reason for hiding this comment

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

Left a bunch of longer explanation comments which are hopefully helpful

@@ -1,3 +1,5 @@
//The name and code suggests this file does the spreadsheet/server stuff
Copy link
Member

Choose a reason for hiding this comment

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

Yeah this file has the function get to fetch the data from the spreadsheet and get it in the correct shape for using in the site, and then also append which turns the response queue into what the server wants. The server is in server/server.js I think and on get requests grabs match data from the sheet as well as the form schema. On post requests it appends each row of data (comes in a list of lists) after the last row of data in a sheet that we specify

src/lib/store.ts Outdated
@@ -3,24 +3,44 @@ import type { Form, Response, Team, Match, Field } from "$lib/types";
import { persisted } from "svelte-local-storage-store";
import { extractGroups } from "./serialize";

//Details of which type of form is being used ???
Copy link
Member

Choose a reason for hiding this comment

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

Yeah this is the full schema of the form (the types here are quite helpful for seeing how the schema is specified, the rendering is quite simple we just render whatever the schema tells us) and it is set when we make the get request to the server

Comment on lines 9 to 11
export const scout = persisted<string>("scout", "");
export const teams = persisted<Record<number, Team>>("teams", {});
export const matches = persisted<Record<number, Match>>("matches", {});
Copy link
Member

Choose a reason for hiding this comment

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

These are for autocomplete when adding a new response, scout saves the name so you don't have to type it in again, and teams and matches is used to have a dropdown for them

src/lib/store.ts Outdated
Comment on lines 16 to 17
//QR code stuff, worry about it later
export const code = persisted<number | null>("code", null);
Copy link
Member

Choose a reason for hiding this comment

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

Same with response this is the id of the QR code we are displaying to be scanned, QR code stuff hopefully shouldn't need any changing

export const code = persisted<number | null>("code", null);

//list of responses which are "active" eg. can be edited (not yet submitted)
Copy link
Member

Choose a reason for hiding this comment

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

Important part is that this is also structured as a map from the response id to the response, which is useful because we use the id in a bunch of places to refer to the response we are using

export const activeResponses = persisted<Record<number, Response>>(
"activeResponses",
{}
);

//something to do with grid, shouldn't be necessary this year
Copy link
Member

Choose a reason for hiding this comment

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

Yeah so the grid won't be necessary but if there is a need for a form field that needs multiple data points then let me know because that is why the grid is much more complicated and needs special handling

src/lib/store.ts Outdated
Comment on lines 31 to 32
//these two look like more QR code stuff
export const keys = persisted<string[]>("keys", []);
Copy link
Member

Choose a reason for hiding this comment

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

Keys is the correct order of the keys for when we need to format the data for appending, you can see how it is populated in setForm(which is called from get to set the form schema) and used in append in sheet.ts

export const fields = persisted<Record<number, Record<string, Field>>>(
"fields",
{}
);

//these two look like more QR code stuff
export const keys = persisted<string[]>("keys", []);
export const scans = persisted<number[]>("scans", []);
Copy link
Member

Choose a reason for hiding this comment

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

Yeah this is just a list of the ids of responses we have already scanned so we don't double scan anything and send it twice to the sheet

export const theme = persisted("theme", "arctos");

//looks like a list of IDs and boolean values for whether or not those forms have errors?
Copy link
Member

Choose a reason for hiding this comment

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

Yeah if a form has any errors then we want to be able to disable the submit button in the home page, key thing here is we initially set the error to true when we create a new response because fields won't populate with default values till the form is actually opened

export const responseQueue = persisted<Response[]>("responseQueue", []);
export const lastGet = persisted("lastGet", 0);

//check is device connected to internet (writable only works online)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like I don't actually use this at all, but both get and append in sheet.ts instead of causing an error when offline will just return false, which for append is important cause only if it returns true do we know it's safe to remove the submitted responses from the response queue

@@ -14,7 +14,7 @@
"devDependencies": {
"@neodrag/svelte": "^2.0.3",
"@sveltejs/adapter-static": "^1.0.1",
"@sveltejs/kit": "1.3.0",
"@sveltejs/kit": "^1.11.0",
Copy link
Member

Choose a reason for hiding this comment

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

If you update sveltekit then you will probably need to update vitepwa/sveltekit as well, does it still build properly with this change?

Copy link
Author

Choose a reason for hiding this comment

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

It still builds properly. I think I have a whole bunch of stuff thats been updated since polaris was made in this commit, but it doesn't seem to have any impact.

Copy link
Member

Choose a reason for hiding this comment

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

ah no worries if something breaks I can push a hotfix for it

Copy link
Member

@gabydd gabydd left a comment

Choose a reason for hiding this comment

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

Added some clarification message me on discord if you have any more questions

src/lib/store.ts Outdated
export const fields = persisted<Record<number, Record<string, Field>>>(
"fields",
{}
);

//This is used for formatting responses before they are sent to the server (not quite sure how though)
Copy link
Member

Choose a reason for hiding this comment

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

This is the order of the keys because JavaScript objects aren't ordered when we take the data from it we want them to be in the correct order for the server

src/lib/sheet.ts Outdated
@@ -201,5 +207,6 @@ const setMatches = (match_array: number[][]) => {
matches.set(match_obj);
};

//Im not really sure what this does but it is necessary for setform
Copy link
Member

Choose a reason for hiding this comment

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

This is a matrix transpose the data is in a 2d array but the columns and rows are incorrect for what we want to do because Google sheets gives us an array of columns but we want an array of rows I think that's what it is atleast haven't touched this code in a while

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.

2 participants