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

Feature/submission upload #249

Closed
wants to merge 15 commits into from
Closed

Conversation

AWerbrouck
Copy link
Contributor

files uploaden kan nu adhv submissions

het zijn maar een paar kleine changes, maar enkele hiervan zijn aan de apifetch dus bekijk deze goed.

@AWerbrouck AWerbrouck marked this pull request as ready for review May 5, 2024 16:03
Copy link
Contributor

@arnedierick arnedierick left a comment

Choose a reason for hiding this comment

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

Op het eerste zicht zien alle changes er goed uit. Het is intuïtief. Wanneer ik echter een indiening wil maken ondervind ik een paar problemen:

  • in de file manager die ik open kan ik geen individuele bestanden selecteren
  • Telkens als ik indien krijg ik een console error, zelfs al is de indiening "succesvol". Ik krijg ook geen confirmatie of de indiening geslaagd is. Probeer die bug op te vangen en kijk eens naar Ant Design's message component
  • Als ik een gemaakte indiening wil downloaden, dan krijg ik een zip map die ik corrupt is. (Windows kan ze niet openen)

Over de apifetch en die technische zaken kan ik niet goed oordelen of je daar wel alles goed doet, dat is meer iets voor @usserwoutV2 om over te oordelen, maar de dingen die ik aanhaal, kan je al aan werken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@usserwoutV2 hier is de bug

@AWerbrouck
Copy link
Contributor Author

@arnedierick kan je nu nog eens proberen, het zou nu terug moeten werken.

@AWerbrouck
Copy link
Contributor Author

ik ga ook nog de upload hier doen

@AWerbrouck AWerbrouck marked this pull request as draft May 9, 2024 16:49
@usserwoutV2
Copy link
Contributor

usserwoutV2 commented May 9, 2024

Ik heb even gecheckt en het ziet er goed uit. Ik heb wel nog enkele typescript errors gezien in de code (ik heb deze even opgelost). Ik heb ook eens Tree naar Tree.DirectoryTree aangepast waardoor het net iets duidelijker is wat files en directories zijn. Verder heb ik de functionaliteit getest en heb geen problemen kunnen vinden.

const projectUrl = new URL(response.data.projectUrl, 'http://localhost:3001');
const courseUrl = new URL(projectUrl.origin + projectUrl.pathname.split('/').slice(0, 3).join('/'), 'http://localhost:3001');
const courseId = courseUrl.pathname.split('/')[2];
const submissionId = response.data.submissionId;
navigate(`/courses/${courseId}/projects/${projectId}/submissions/${submissionId}`);

Ik weet niet wat je hier juist doet, maar dit zal waarschijnlijk problemen veroorzaken als het op de server gehost wordt (omdat we dan niet meer in localhost zitten). Als je de courseId wilt krijgen dan kan je dat best via const course = useCourse() of via const {courseId} = useParams(). Om naar de route te gaan gebruik je best de AppRoutes.SUBMISSION.replace(':courseId', '...').replace(':projectId', ...).replace(':submissionId',...) in plaats van /courses/${courseId}/projects/${projectId}/submissions/${submissionId}.

@arnedierick
Copy link
Contributor

Het is inderdaad al beter. Ik kan nu ook individuele bestanden zien en uploaden. De download werkt nu ook!
Er zijn wel nog steeds enkele dingen die ik gefixt zou willen hebben voor we de PR kunnen mergen:

  1. Bij een indiening krijg ik nog altijd een axioserror bij Submit.tsx:36. Hiervoor zou je misschien best eens de hulp van Wout vragen. De indiening lijkt wel geslaagd, maar toch krijg ik deze error
  2. Er is nog altijd niets van confirmatie of de indiening geslaagd is of niet
  3. Kleinigheid, maar als er een bestand geselecteerd is voor indiening, staat er op de knop "Folder indienen". Dit kan je gewoon vervangen door "Indienen"

@AWerbrouck AWerbrouck closed this May 13, 2024
@AWerbrouck AWerbrouck deleted the feature/submissionUpload branch May 13, 2024 15:26
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.

3 participants