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

Maak projecten automatisch zichtbaar op een bepaald tijdstip #269

Merged
merged 10 commits into from
May 19, 2024

Conversation

arnedierick
Copy link
Contributor

De mogelijkheid voor course_admins is toegevoegd in de frontend om projecten automatisch zichtbaar te maken voor studenten na een gekozen uur.

Dit werkt en is getest bij zowel de creatie als aanpassing van een project

Dit is een oplossing voor #267

Ik ga de PR open maken wanneer ik extra gebruiksvriendelijkheid heb toegevoegd aan de projectTable voor course_admins, zodat zij deze nieuwe info duidelijk kunnen zien

@arnedierick arnedierick marked this pull request as ready for review May 18, 2024 14:53
@arnedierick
Copy link
Contributor Author

Alles is geïmplementeerd, ik denk dat dit klaar is voor review!
Ik zie wel dat er een falende test is. Omdat ik niets heb aangepast in de backen, denk ik dat dit niet mijn fout is.

@Aqua-sc
Copy link
Contributor

Aqua-sc commented May 18, 2024

Alles is geïmplementeerd, ik denk dat dit klaar is voor review!

@usserwoutV2 oké als ik de review aan jou overlaat?

Ik zie wel dat er een falende test is. Omdat ik niets heb aangepast in de backen, denk ik dat dit niet mijn fout is.

De lijnen code zijn uitgecomment van de imageExists en doen een test falen. De imageExists is normaal gefixt dus eens mergen met dev zou de test moeten doen slagen

@SELab-2 SELab-2 deleted a comment from Aqua-sc May 18, 2024
Copy link
Contributor

@usserwoutV2 usserwoutV2 left a comment

Choose a reason for hiding this comment

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

Er zijn nog 2 kleine aanpassingen nodig + nog eens mergen met dev voor de testen te fixen.

name="visibleAfter"
>
<DatePicker
showTime
Copy link
Contributor

Choose a reason for hiding this comment

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

Hier moet nog allowClear staan omdat je het veld leeg mag laten.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik heb het erbij gezet maar AllowClear is true by default

useEffect(() => {
setIsVisible(visible)
if (visible && savedVisibleAfter) {
form.setFieldsValue({ visibleAfter: null })
Copy link
Contributor

Choose a reason for hiding this comment

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

Ik denk dat het gemakkelijker is om dit te doen net voor je de POST/PUT request maakt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik heb deze logica verplaatst naar de handleCreation functies van EditProject en ProjectCreate en de overbodige code uit GeneralFormTab verwijderd

@arnedierick
Copy link
Contributor Author

De requested changes zijn aangepast. Ik denk dat de merge nu wel zou mogen doorgaan

@arnedierick arnedierick requested a review from usserwoutV2 May 18, 2024 22:10
@arnedierick
Copy link
Contributor Author

Alles is geïmplementeerd, ik denk dat dit klaar is voor review!

@usserwoutV2 oké als ik de review aan jou overlaat?

Ik zie wel dat er een falende test is. Omdat ik niets heb aangepast in de backen, denk ik dat dit niet mijn fout is.

De lijnen code zijn uitgecomment van de imageExists en doen een test falen. De imageExists is normaal gefixt dus eens mergen met dev zou de test moeten doen slagen

De test faalt nog altijd zelfs na een merge met Dev. De falende test is testCheckForTestUpdate bij het pad "Invalid dockerImage".
Ik denk dat ik dit best aan jou overlaat @Aqua-sc

@Aqua-sc
Copy link
Contributor

Aqua-sc commented May 18, 2024

@arnedierick

// This returns false if the image isn't pullt yet! FIX PLS
(dit moet uit comments)

@arnedierick
Copy link
Contributor Author

De test werkt nu, ik ga mergen

@arnedierick arnedierick merged commit 09fc569 into frontend May 19, 2024
1 check passed
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.

5 participants