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

1125 implement permission system algorithm #1126

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

emsoraffa
Copy link

see #1125 for info

@emsoraffa emsoraffa added the backend Task related to backend. label May 14, 2024
@emsoraffa emsoraffa self-assigned this May 14, 2024
@emsoraffa emsoraffa linked an issue May 14, 2024 that may be closed by this pull request
Copy link
Member

Choose a reason for hiding this comment

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

wow, hva skjer nå?

Copy link
Contributor

Choose a reason for hiding this comment

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

UAxa8

Copy link
Author

Choose a reason for hiding this comment

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

O(n^3)

Copy link
Member

Choose a reason for hiding this comment

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

Her må jeg dessverre inn som kritisk pang

Copy link
Member

Choose a reason for hiding this comment

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

Hva er bakgrunnen til alt dette?

Copy link
Member

@emilte emilte May 14, 2024

Choose a reason for hiding this comment

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

Beklager, er ikke meningen å være kjip. Jeg virker strengere over tekst enn det som er meningen, er vanskelig å unngå. Det er sikkert en kul algoritme Emil, har ikke satt meg så dypt inn i koden ennå. Jeg frykter bare at dette er et villspor. Ved første øyeblikk virker det ikke lesbart eller intuitivt. Når matriser og grafer trekkes inn blir jeg noe usikker. Å treffe O(N^3) må jeg se nærmere på, tror ikke en løsning trenger å være mer enn O(N) og O(1).
Hvor lenge har dere jobbet med dette? Mathias startet på dette i fjor, men ser det er gjort en helomvending jeg ikke helt har fått med meg. Tror det hadde vært lurt å ta med meg i loopen her tidligere. Jeg har tilfeldigvis jobbet masse med rollesystem det siste året. Å gå helt offroad med noe som ikke er kompatibelt med Django har svært stor risiko for komplikasjoner senere, og jeg tror ikke vi klarer å tenke ut alt som kan skje. Jeg er usikker på hvordan dette skalerer på sikt. Men jeg trenger å forstå mer om hvordan dere har kommet frem til denne løsningen først. Jeg forstår ikke per nå helt hvordan dette kobles opp mot brukersystem og hvordan det skal være dynamisk og fleksibelt. Har dere lest Django sin dokumentasjon om Backends? Systemet deres kan faktisk modifiseres ganske mye etter behov. Vi bør ha svært gode grunner til å divergere i en såpass kritisk del av koden, og trestruktur-behovet trenger jeg mer innsikt i. Har dere en konkret use-case dere kan beskrive for meg? Kanskje finnes det løsninger der arv kan unngås 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Det kan også hende at jeg har misforstått noe her.

Copy link
Member

Choose a reason for hiding this comment

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

Funker denne løsningen på tabell-nivå, kolonne-nivå, rad-nivå?

Copy link
Author

Choose a reason for hiding this comment

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

Har ikke mye erfaring med Django Groups så skal ikke uttale meg om hva som er eller ikke er mulig, den avgjørelsen ble tatt før jeg begynte å se på permissions. Tror dette heller burde diskuteres på arbeidskveld enn her. Som nevnt har jeg laget en pdf som forklarer mye bedre enn jeg får til her.

Copy link
Member

Choose a reason for hiding this comment

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

Er nok lurere det ja.

backend/root/utils/compute_permissions.py Outdated Show resolved Hide resolved
backend/root/utils/compute_permissions.py Outdated Show resolved Hide resolved
backend/root/utils/compute_permissions.py Outdated Show resolved Hide resolved
backend/root/utils/compute_permissions.py Outdated Show resolved Hide resolved
Comment on lines 3 to 24
class Permissions:
def __init__(self, dictionary: dict):
self.dict = {k: set(v) for k, v in dictionary.items()}
self.index_array = list(dictionary.keys())

def __getitem__(self, key: int) -> str:
return ''.join(sorted(self.dict[self.index_array[key]]))

def __str__(self) -> str:
return str({k: ''.join(sorted(v)) for k, v in self.dict.items()})

def get(self, index: int) -> set:
return self.dict[self.index_array[index]]

def set(self, key: str, value: str) -> None:
if key in self.dict:
self.dict[key].update(value)
else:
raise KeyError("Key not found in permissions dictionary")

def get_index_array(self) -> list:
return self.index_array
Copy link
Contributor

Choose a reason for hiding this comment

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

Bruk mer forklarende navn ikke dictionary, self.dict, k, v, osv

Copy link
Author

Choose a reason for hiding this comment

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

vanskelig å gjøre det så mye mer forklarende. Kan kanskje revurdere navn på klassen, er egt bare en datastruktur som er et dictionary med "set" values og mulighet for indeksering.

Copy link
Contributor

Choose a reason for hiding this comment

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

Docstrings da, noe som er litt mer forklarende over hva som skjer og hvorfor

@emsoraffa emsoraffa requested a review from tiniuspre May 14, 2024 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Task related to backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement permission system algorithm
5 participants