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

add and edit matches manually #1534

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open

add and edit matches manually #1534

wants to merge 19 commits into from

Conversation

varCepheid
Copy link
Collaborator

@varCepheid varCepheid commented Nov 9, 2023

Admins and super-admins can create new matches for an event manually and edit existing matches. The editable fields are name, date & time, team list, and score.

@varCepheid varCepheid added enhancement Adds a new feature or meets a request. wip priority This needs to be addressed before the current changes can be merged to main. backend Changes need to be made in the backend code. labels Nov 9, 2023
@varCepheid varCepheid self-assigned this Nov 9, 2023
@varCepheid varCepheid marked this pull request as draft November 15, 2023 01:13
@varCepheid varCepheid removed wip backend Changes need to be made in the backend code. labels Nov 15, 2023
@varCepheid varCepheid added the question Further information is requested. label Dec 14, 2023
@varCepheid
Copy link
Collaborator Author

@calebeby When I try to test the match-creator form in the deploy preview, it throws a red Alert popup, but it doesn't tell me what the problem is. Can you look through the code and tell me if there's anything that should cause a problem?

@varCepheid
Copy link
Collaborator Author

The same error occurs when saving the match-editor form.

fixed back arrow on match-editor
added match-editor link on match pages
@calebeby
Copy link
Member

@varCepheid When I look in devtools it shows me that the PATCH request did not go through; the server returned a 405 Method Not Allowed:

image

Based on the API Docs for the backend, it looks like the server does not support updating matches, only creating new ones. So we are missing a backend method that you (or someone) will need to send a PR for.

image

@calebeby
Copy link
Member

calebeby commented Dec 19, 2023

Looks like there is an undocumented PUT for updating matches. Maybe you can use that instead of PATCH: https://github.com/Pigmice2733/peregrine-backend/blob/develop/internal/server/routes.go#L40C26-L40C26

And the POST method for /events/{eventKey}/matches in the documentation is not implemented. You should be able to use the PUT directly to make a match instead of using the POST.

Maybe it would be good to update the OpenAPI docs in https://github.com/Pigmice2733/peregrine-backend/blob/develop/internal/server/openapi.yaml

Feel free to send a PR!

cc @brendanburkhart

@varCepheid
Copy link
Collaborator Author

The forms are going through fine now. I've fixed all the problems I could find except that when I go into the match editor, it will always display (in the placeholder attribute) the current date-time instead of the match's date-time. It should only do that if it is unable to load the match or the match's time property in the code, but it's weird to me that it wouldn't be able to load that information, since it displays correctly on the match card. Any ideas?

@varCepheid varCepheid requested a review from calebeby January 1, 2024 01:29
@calebeby
Copy link
Member

calebeby commented Jan 6, 2024

@varCepheid It is because the match variable is null for a bit while useMatchInfo is fetching the match info. During that time, matchDate gets set to the current time, and then day and time get initialized with those values. You did not write any code to update day and time when matchDate changes (/ is loaded). You could use a useEffect for this?

@varCepheid varCepheid marked this pull request as ready for review January 16, 2024 02:00
@varCepheid varCepheid removed the question Further information is requested. label Jan 16, 2024
@varCepheid
Copy link
Collaborator Author

Added the useEffect call and fixed the problem that I mentioned in issue #1536.

@varCepheid varCepheid removed the priority This needs to be addressed before the current changes can be merged to main. label Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adds a new feature or meets a request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants