(Post feedback version) GS Onboarding#22
(Post feedback version) GS Onboarding#22GabrielGuirguis wants to merge 7 commits intoUWOrbital:mainfrom
Conversation
frontend/src/display/table.tsx
Outdated
| deleteCommand(id) | ||
| } catch (error) { | ||
| console.error(error) | ||
| throw error |
There was a problem hiding this comment.
We shouldnt be throwing inside the component. Instead, display an error popup that we couldnt successfully delete the command
frontend/src/input/command_input.tsx
Outdated
| // TODO:(Member) Submit to your post endpoint | ||
| } | ||
| const [selectedCommandId, setSelectedCommandId] = useState<string>( | ||
| mainCommands[0].id.toString() |
There was a problem hiding this comment.
Main commands will be empty on first load thus, the 0th element will be undefined, resulting in an underfined reference error
Yarik-Popov
left a comment
Yarik-Popov
left a comment
There was a problem hiding this comment.
Have you tested your frontend locally when building it? None of the functionality works properly
frontend/src/input/command_input.tsx
Outdated
| params: commandParams, | ||
| }; | ||
|
|
||
| createCommand(command); |
There was a problem hiding this comment.
When creating a command, the page doesn't display the command after submitting. The workaround is to refresh the page
| </div> | ||
| <input /> {/* TODO: (Member) Add input handling here if the selected command has a param input*/} | ||
| <button onClick={handleSubmit}>Submit</button> | ||
| <input onChange={(e) => setCommandParams(e.target.value)} />{" "} |
There was a problem hiding this comment.
Each command can have different types and number of arguments and the frontend should be able to handle that. They are encoded in the mainCommands
frontend/src/display/table.tsx
Outdated
| const handleDelete = (id: number) => { | ||
| return () => { | ||
| try { | ||
| deleteCommand(id) |
There was a problem hiding this comment.
This doesn't properly delete a command
frontend/src/display/command_api.ts
Outdated
| */ | ||
| export const deleteCommand = async (id: number): Promise<CommandListResponse> => { | ||
| try { | ||
| const { data } = await axios.delete(`${API_URL}/${id}`) |
There was a problem hiding this comment.
This is wrong. You need to delete at the commands route
| response = await call_next(request) | ||
|
|
||
| duration = perf_counter() - start | ||
| logger.info(f"Response sent in {duration} seconds to Request params: {request.path_params}") |
There was a problem hiding this comment.
need to print the datetime of request
backend/data/data_models.py
Outdated
| if self.params is None and self.format is None: | ||
| return self | ||
| elif isinstance(self.params, str) and isinstance(self.format, str) and self.params.count(",", 0, -1) == self.format.count(",", 0, -1): | ||
| return self |
| const handleDelete = (id: number) => { | ||
| return () => { | ||
| try { | ||
| deleteCommand(id); | ||
| window.location.reload(); | ||
| } catch (error) { | ||
| alert(`Failed to delete command with id ${id}`); | ||
| } |
kepler452b123
left a comment
kepler452b123
left a comment
There was a problem hiding this comment.
LGTM, send email to be added to Notion/Github
3 participants
Purpose
Completed the GS on-boarding task. Include a screenshot of the front-end of the application.
New Changes
Testing
Outstanding Changes