water / mainline

Core web service
GNU Affero General Public License v3.0
9 stars 0 forks source link

Ska verkligen AssistantRegisteredToGivenCourse bara ha EN LabHasGroup? #157

Closed jesjos closed 12 years ago

jesjos commented 12 years ago

Personligen känner jag att den borde ha många LabHasGroups? En assistent måste väl kunna rätta många labbar?

Feedback ASAP, pls

@oleander @spontus @anjonas @karinsofia @Tarrasch

karinsofia commented 12 years ago

Ja, låter som att något gått lite tokigt där.

Den 3 maj 2012 10:42 skrev Jesper Josefsson < reply@reply.github.com

:

Personligen känner jag att den borde ha många LabHasGroups? En assistent måste väl kunna rätta många labbar?

Feedback ASAP, pls

@oleander @spontus @anjonas @karinsofia @Tarrasch


Reply to this email directly or view it on GitHub: https://github.com/water/mainline/issues/157

jesjos commented 12 years ago

Det känns även som att modellen AssistantRegisteredToGivenCoursesLabHasGroup är onödig. Det är ju en ren join-tabell?

Jag föreslår följande åtgärder:

jesjos commented 12 years ago

Justja. Det ska förstås vara en HABTM-relation.

Tarrasch commented 12 years ago

Det kan vara en kvarleva av tanken vi hade på att vi ska strikt bestämma vilken lhg en handledare ska rätta i vilken ordning o.s.v.. Det är ju bara fel på alla sätt. Jag föreslår nog att vi gör som du säger @jesjos, fast jag har riktigt inte koll om man ändå inte alltid brukar ha en modell för alla tabeller? Har man inte det?

Men svar på den viktiga frågan är nog att en handledare givetvis har flera lhgs som den rättar.

jesjos commented 12 years ago

Rena join-tabeller för HABTM-relationer har man i regel inte modeller för. När man har definierat relationen i modellen så letar Rails efter en tabell med ett namn som matchar konventionen (alfabetisk ordning, pluralisform, snail casing). Vi har några join-tabeller som även innehåller lite extra information, typ LabHasGroup. Då är det nödvändigt med en modell.

Tarrasch commented 12 years ago

Är det verkligen many-to-many? Kan flera handledare verkligen rätta samma labb? Det är iufersig inte mig alls emot, men visst är det innebörden?

jesjos commented 12 years ago

Du har rätt, det är jag som svamlar! has_many är bättre. Men i så fall så borde vi ha en assistant_registered_to_given_course_id-kolumn i lab_has_group-tabellen, eller?

jesjos commented 12 years ago

Nu när jag funderar på saken så kanske det är en onödig restriktion att bara ha en handledare som har rätt att rätta en labb. Kanske vi får skilja på koncepten att vara rättarEN för en labb att FÅ rätta en labb? D.v.s. Assistant has_many :lab_has_groups men det räcker med att vara Assistant på rätt GivenCourse för att få rätta labbar i den kursen? Iof blir det mera logik att hålla reda på...

Konsekvensen av has_many som jag ser dom:

Tarrasch commented 12 years ago

Ja, så borde det vara va?

Alternativt, vad skulle hända om man har bara assistant_id i lab_has_group? Bara funderar vad det skulle innebära.

Ett problem däremot, då få vi ju en null-value approach troligen såvida vi inte vet vid skapandet av en lhg vem som ska rätta den.

jesjos commented 12 years ago

Vad menar du med alternativt i ditt andra stycke, det var ju en sådan lösning som jag presenterade just?

Ja, null blir ett problem. Men å andra sidan så kanske vi borde ha en invariant som ser till så att en lhg får en assistant när den skapas?

oleander commented 12 years ago

@jesjos Din senaste lösning är en null-value-approach på den redan existerande implementeringen.

jesjos commented 12 years ago

@oleander Inte om vi har en invariant.

jesjos commented 12 years ago

@oleander Den nuvarande implementering stöder bara en LabHasGroup per AssistantRegisteredToGivenCourse, så implementeringarna är inte funktionellt ekvivalenta.

En alternativ lösning är att ha en join-tabell med assistant_registered_for_given_course_id och lab_has_group_id där vi ser till så att lab_has_group_id är unik. Vet inte riktigt hur man uttrycker den lösningen i Rails-relationer. Kanske det ändå krävs en join-tabells-entitet som tar hand om valideringen.

Tarrasch commented 12 years ago

Kan nån klargöra vad den nuvarande lösningen är? Var det inte så att den bara tillät en assistent att rätta en labb?

jesjos commented 12 years ago

Den nuvarande lösningen är att en AssistantRegisteredForGivenCourse kan ha en och bara en LabHasGroup åt gången.

jesjos commented 12 years ago

Det vore kul om vi kunde bestämma något i denna fråga.

Tarrasch commented 12 years ago

Jag tycker inte vi ska ha n:m relation mellan lhgs o rättare. Vill du att flera assistenter ska kunna rätta, fast endast en vara riktig rättare tycker jag vi snarare implementerar att en är riktig rättare och alla andra assistenter i kursen kan fiffla med repon.

jesjos commented 12 years ago

Mitt senaste förslag säkerställer att lhg har en och endast en rättare. En rättare har flera lhgs. Alltså en jointabell där lhg_id är unik (som sagt inte säker på den exakta implementationen). Sen kan vi göra som du säger att alla andra assistenter kan läsa repon.

Tarrasch commented 12 years ago

Låter bra det förslaget. Vad säger Linus?

oleander commented 12 years ago

@Tarrasch Har läst ditt inlägg men har inte tid att svara med ingående detaljer just nu. Återkommer i slutet av veckan.

jesjos commented 12 years ago

Jag förstår om det är stressigt för dig Linus.

Slutet av veckan brukar betyda typ nu för dom flesta, men jag antar att LInus menar typ på söndag.

Så länge tycker jag inte att vi väntar.

Vi är trots allt 5 andra i gruppen som borde kunna ta ett beslut.

Jag vill därför ha in feedback från @spontus @anjonas @karinsofia

oleander commented 12 years ago

Hehe, såg precis de här (bugg?) på min dashboard.

PS

Jag får dock känslan av att de skulle kunna vara på riktigt :)

@jesjos Jag har tyvärr upp till öronen med saker som ska göras tills på måndag. Jag tänkte sätta mig in i issue-detaljerna på söndag. Övriga i gruppen kanske kan flika så länge.

Tarrasch commented 12 years ago

@oleander, mja, var mest jag som var klantig och tröck submit 4 gånger, fast sidan var irresponsiv då och tillåt mig klicka utan att den "visade" att mitt inlägg verkligen postats.

anjonas commented 12 years ago

Jag tycker också att vi bara skapar en join-tabell mellan LabHasGroup och AssistantRegisteredForGivenCourse, den lösningen känns mest naturlig, iom att en inläming ska bara ha en rättare.

Tarrasch commented 12 years ago

Bara så jag inte missuppfattat nått: Join-tabell innebär väll en n:m-relation i databasen, fast vi ser till att det egentligen blir 1:m (1 handledare : m labbar) genom att vi lägger till en validering i join-tabellen som ser till att det inte redan finns en handledare för en labb.

jesjos commented 12 years ago

Det som Arash skriver stämmer. Jonas, du fick det lite om bakfoten.

5 maj 2012 kl. 08:39 skrev Arash Rouhanireply@reply.github.com:

Bara så jag inte missuppfattat nått: Join-tabell innebär väll en n:m-relation i databasen, fast vi ser till att det egentligen blir 1:m (1 handledare : m labbar) genom att vi lägger till en validering som ser till att det inte redan finns en handledare för en labb.


Reply to this email directly or view it on GitHub: https://github.com/water/mainline/issues/157#issuecomment-5524930

jesjos commented 12 years ago

Jag har slängt ihop en möjlig lösning. Fixar pull request strax.

jesjos commented 12 years ago

Såja. Ta gärna en titt på pull requesten.