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

Problem med role som scope #151

Open
jesjos opened this issue Apr 26, 2012 · 9 comments
Open

Problem med role som scope #151

jesjos opened this issue Apr 26, 2012 · 9 comments

Comments

@jesjos
Copy link
Member

jesjos commented Apr 26, 2012

Att hämta role på basis av scope i url:er är bra på många sätt, men ställer även till problem.

Exempel:

  • Alla path helpers måste ha en roll som parameter. Detta börjar kännas segt.
  • Alla routes passar inte under en scope. Till exempel Session#new känns som att det ska ligga utanför roll-scopet. Men eftersom många path helpers kräver en roll som parameter, så pajar alla dom när man går in på en sida utan roll-scope. Nu var det inte ett klockrent exempel, men poängen är att om vi i framtiden vill ha en icke-scopad sida för inloggade användare så kommer vi att till exempel behöva en separat navigeringspartial bara för den.

Jag tycker att detta talar för att vi inför current_role som en variabel i session eller i en cookie istället. Actions som inte är olika beroende på vilken roll vi har kan då strunta i att läsa den variabeln. Pathhelpers blir enklare att använda eftersom roll-hanteringen läggs utanför url:en.

Vad säger ni?

@Tarrasch
Copy link
Contributor

Om jag förstått det rätt är det inte restful ifall vi har rollen sparat i en sessionvariabel. Länken skriver lite om restful och varför det är bra.

Jag tycker däremot det är minst lika oroande att alla path_helpers måste ha en roll som parameter, mycket riktigt är det ju jättemånga sidor där man inte har en roll mer än att man är en användare. Tror du inte det finns någon lösning på detta då bara? Tyvärr kan jag inte hjälpa så mycket med det tekniska, eftersom jag knappt vet vad pathhelpers är för något i rails. Men om vi kunde lösa detta istället för att gå över till sessionlösningen skulle jag vara jättenöjd. Det skulle ju också fixa båda dina punkter.

@Tarrasch
Copy link
Contributor

Förresten, nog bäst att göra en "call to arms": @oleander @spontus @karinsofia @anjonas !

@jesjos
Copy link
Member Author

jesjos commented Apr 28, 2012

EDIT: rättade den genererade pathen

Pathhelpers används för att generera en länk till en viss resource. Till exempel course_lab_group_labs_path("student", @course, @lab_group) genererar /student/courses/1/lab_groups/1/labs, då antar vi att kursen och gruppen hade id 1.

Pathhelpers genereras från routes.rb med hjälp av metaprogrammering. Jag antar att role-problemet går att lösa genom att typ monkeypatcha alla dom genererade metoderna så att dom använder role när det behövs men ignorerar det när det inte behövs. Men det har jag inte nog med kompetens för att göra.

En fundering gällande att köra restful role:

  • Rollen sparas i alltså bara i urlen.
  • I navigeringsfältet vill man typiskt ha länkar till typ labs, courses, kanske submission. Dessa länkar är role-scopade.
  • Antag att användaren navigerar till en sida som inte existerar i ett role-scope.
  • För vilken roll ska vi då generera länkarna i navigeringsfältet? Eller ska icke-scopade sidor aldrig kunna innehålla scopade länkar?

@Tarrasch
Copy link
Contributor

Angående din fundering: Jag tror vi kan köra på att icke-scopade sidor bara visar icke-scopade länkar. Jag oroar mig inte så mycket för det, för jag tror det är väldigt sällan man går från en scopad till en icke-scopad sida. När skulle det vara? Enda icke-scopade sidan jag kommer på är den du skriver upp ditt riktiga namn och sån user-information. Finns väll inget som är gemensamt för alla tre roler i övrigt eller?

Förresten, tror du inte vi bara kan ha två uppsättningar path_hepers, alltså en som är för scopade sidor och en som är för ickescopade? Dom scopade t.ex. role_course_lab_group_labs_path börjar med role_. Ickescopade, typ user_settings, börjar med ingenting alls, alternaivt börjar med user_. Som sagt har jag inte använt path_helpers själv, men jag försöker brainstorma lite ändå. :)

@jesjos
Copy link
Member Author

jesjos commented Apr 28, 2012

Ja, det kan nog vara så att vi inte behöver scopade länkar i icke-scopade sidor. Men jag är inte lika säker som du.

Gällande path helpers så stämmer det att bäst vore det om vi kunde ta alla routes som ligger innanför role-scopet och se till att deras role fylls i automatiskt såvida man inte manuellt ger en roll som argument till helpern. Men jag har som sagt ingen aning om hur man skulle göra det.

@Tarrasch
Copy link
Contributor

Ja, vi får nog fundera på det och låta det smälta lite innan vi beslutar huruvida man länkar mellan scopesidtyperna.

hmm. Vi kanske ska höra ifall @oleander redan har en lösning på detta, annars känns det som att det borde gå att ordna rätt enkelt (så som man brukar säga om allt :)).

@oleander
Copy link
Member

Vad sägs om de här?

app/helpers/application_helper.rb

module ApplicationHelper
  #
  # Adds {params[:role]} to every path 
  # that ends with _path and starts with _role
  # If no role is found in the given url, try to
  # find a role in any of the given arguments ({args})
  #
  # Example 1:
  #  Current url: /student/labs
  #  role_course_lab_group_labs_path(course, group)
  #  => "/student/courses/1/lab_groups/1/labs"
  #
  # Example 2:
  #  Current url: /examiner/labs
  #  role_course_lab_group_labs_path(course, group)
  #  => "/examiner/courses/1/lab_groups/1/labs"
  #
  # Example 3: (User is a student in {course}):
  #  Current url: /labs
  #  role_course_lab_group_labs_path(course, group)
  #  => "/student/courses/1/lab_groups/1/labs"
  #
  # @method Symbol Non existing path
  # @args Array<?> A list of arguments
  # @return String Absolute path (wihout current host) to {method}
  # @todo Write some specs
  #
  def method_missing(method, *args, &block)
    return super unless method =~ /^role_.+_path$/

    # Do we have a role?
    # If not, does any of the given arguments contain one?
    params[:role] ||= args.select{ |a| a.is_a?(GivenCourse) }.first.
      try(:role_for_user, current_user)

    unless params[:role]
      raise "I'm not sure what to do"
    end

    # Remove _role in front of {method} and call helper method
    send(method.to_s.gsub(/^role_/, ""), params[:role], *args)
  end
end

app/models/given_course.rb

class GivenCourse < ActiveRecord::Base
  #
  # @current_user User
  # @return String Current role for @current_user
  # @todo Implement and write some specs
  #
  def role_for_user(current_user)
    "student"
  end
end

oleander added a commit that referenced this issue Apr 30, 2012
]

An empty GivenCourse#role_for_user was also added
@jesjos
Copy link
Member Author

jesjos commented Apr 30, 2012

Jag gillar lösningen, ser fräsch ut vid första anblick!

@jesjos
Copy link
Member Author

jesjos commented May 2, 2012

Vid diskussioner kom vi fram till detta (kopierat från mötesprotokoll):

  • Vi använder Linus metaprogrammeringsmetod för att lägga till scope när det finns och behövs på URL:er
  • En oscopad dashboard innehåller information om alla dina roller
  • Den oscopade dashboarden är uppdelad efter roll, länkar inom en roll är scopade
  • Oscopade URL:er i navbar när man är i den oscopade dashboarden
    • När man trycker på en oscopead-url måste man göra ett val OM man har mer än en roll
    • Antagligen kan vi lösa detta automagiskt med Linus' metaprogrammeringsmetod

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants