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

adding course year to backend #225

Merged
merged 13 commits into from
Apr 27, 2024
Merged

adding course year to backend #225

merged 13 commits into from
Apr 27, 2024

Conversation

AWerbrouck
Copy link
Contributor

Kunnen jullie eens zien of ik iets over het hoofd heb gezien, de testen runnen maar het kan zijn dat dit mss iets in de frontend breekt ofzo.

@AWerbrouck AWerbrouck linked an issue Apr 25, 2024 that may be closed by this pull request
@Aqua-sc
Copy link
Contributor

Aqua-sc commented Apr 25, 2024

Ik kan het niet uitvoeren want zit op de bus naar huis, ik denk da het er oke uit ziet voorlopig. Je zou wel nog de controllers moeten aanpassen dat dit veld ook teruggegeven wordt waar nodig; het veld opgeslagen wordt bij POST/PATCH/PUT, check dat het niet null is bij POST/PUT, misschien nog iets dat ik over het hoofd zie

@AWerbrouck
Copy link
Contributor Author

Ik kan het niet uitvoeren want zit op de bus naar huis, ik denk da het er oke uit ziet voorlopig. Je zou wel nog de controllers moeten aanpassen dat dit veld ook teruggegeven wordt waar nodig; het veld opgeslagen wordt bij POST/PATCH/PUT, check dat het niet null is bij POST/PUT, misschien nog iets dat ik over het hoofd zie

Ahn ja dat moet ik nog doen, dat had ik over het hoofd gezien, ik kan vnvd niet meer maar morgen ochtend doe ik dat nog

@@ -1,6 +1,7 @@
package com.ugent.pidgeon.postgre.models;

import jakarta.persistence.*;
import jakarta.persistence.criteria.CriteriaBuilder;
Copy link
Contributor

Choose a reason for hiding this comment

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

Deze import lijkt overbodig

(3,'History 101', 'World History Overview'),
(4,'Computer Science 101', 'Introduction to Computing'),
(5,'English 101', 'English Literature');
INSERT INTO courses (course_id,course_name, description, coures_year) VALUES
Copy link
Contributor

Choose a reason for hiding this comment

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

typfout, ik pas het aan

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.

Alles lijkt in orde, je gaat wel de controllerfuncties ook nog moeten uitbreiden met het nieuwe veld voordat we naar dev kunnen pushen, zoals @Aqua-sc al zei.

@AWerbrouck
Copy link
Contributor Author

thx @arnedierick

@AWerbrouck AWerbrouck marked this pull request as ready for review April 26, 2024 09:55
@Aqua-sc
Copy link
Contributor

Aqua-sc commented Apr 26, 2024

@usserwoutV2 ik weet niet hoeveel arthur al met jouw hierover gecommuniceerd heeft maar indien dit nog niet gebeurt zou zijn:

  • Elk vak heeft nu een veld "year", in de POST/PATCH/POST responses kan dit ook meegegeven, een mogelijke body van POST is dus:
{
	"name": "Elektrotechniek",
	"description": "...",
	"year": 2023
}
  • Overal waar één specifiek vak met alle info gereturned wordt (bij /courses/{courseId} PUT/PATCH/POST/GET en wanneer een vak gejoined is /courses/{courseId}/join</{courseKey} POST. De volledige response body wordt dan bv:
{
	"courseId": 1,
	"name": "TestUpdate",
	"description": "TestPutDescription",
	"teacher": {
		"name": "Inti Danschutter",
		"email": "[email protected]",
		"userId": 1
	},
	"assistants": [],
	"memberUrl": "/api/courses/1/members",
	"joinUrl": "/api/courses/1/join/aa904a29-6f89-4473-999a-e5477be96a3d",
	"joinKey": "aa904a29-6f89-4473-999a-e5477be96a3d",
	"archivedAt": null,
	"createdAt": "2024-04-26T14:24:30.118749Z",
	"year": 2023
}

Indien dit oké is voor de frontend (en het bv. niet op andere plaatsen ook gereturned moet worden) mag deze branch van mij gemerged worden

@usserwoutV2
Copy link
Contributor

Ik gebruik nu ik archivedAt & createdAt om het jaar weer te geven. Year veld is in dat geval wel handiger. Het veld heb ik nodig waar we nu archivedAt & createdAt hebben staan, dus ook /api/courses. In principe hebben we dan niet meer createdAt veld nodig.

@Aqua-sc
Copy link
Contributor

Aqua-sc commented Apr 27, 2024

Okay, ik heb het jaar nu ook toegevoegd aan /courses (kheb createdAt ook gwn laten staan was makkelijker). Ik ga meteen mergen want dan kan ik zorgen dat het jaar ook gekopieerd wordt in #230 zodat deze uit draft kan

@Aqua-sc Aqua-sc merged commit f17d6e9 into development Apr 27, 2024
1 check passed
@AWerbrouck AWerbrouck deleted the feature/224-jaartallen branch May 23, 2024 17:37
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.

Jaartallen toevoegen aan vakken
4 participants