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

Add membershipstatus #154

Merged
merged 2 commits into from
Feb 19, 2024
Merged

Add membershipstatus #154

merged 2 commits into from
Feb 19, 2024

Conversation

rrooij
Copy link
Contributor

@rrooij rrooij commented Feb 18, 2024

The members lacked a status field which made it hard to keep track of whether members were ex-members, members, expelled, cancelled their membership etc.

This introduces arbitrary fields of membership statuses that are being able to be set by the administrators.

The migration makes a single membership type in advance which is called "Lid" (member) and everyone by default will be made member and get access to the login.

You can make additional membership types which can't login to the system at all.

The members lacked a status field which made it hard
to keep track of whether members were ex-members, members,
expelled, cancelled their membership etc.

This introduces arbitrary fields of membership statuses
that are being able to be set by the administrators.

The migration makes a single membership type in advance
which is called "Lid" (member) and everyone by default
will be made member and get access to the login.

You can make additional membership types which can't
login to the system at all.
@rrooij rrooij mentioned this pull request Feb 18, 2024
3 tasks
public function setName(string $name): void { $this->name = $name; }

public function getAllowedAccess(): bool { return $this->allowedAccess; }
public function setAllowedAccess(bool $allowedAccess) { $this->allowedAccess = $allowedAccess; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're missing the void return type of the function here.

return $this->currentMembershipStatus;
}

public function setCurrentMembershipStatus(?MembershipStatus $membershipStatus) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing void return type here.

/** @see UserInterface */
public function getUsername(): string { return $this->id; }

/** @see UserInterface */
public function getRoles(): array {
$roles = $this->roles;
$roles[] = 'ROLE_USER';
if ($this->getCurrentMembershipStatus() === null || $this->currentMembershipStatus->getAllowedAccess()) {
$roles[] = 'ROLE_USER';
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the behaviour if the user has 0 roles? Will that not break things?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user always has the Anonymous/Public user role. And in the case of no roles, it will either deny access or add one of the roles to the array (as the roles property of Member is always an array by default)

@rrooij rrooij merged commit 35cfe8f into main Feb 19, 2024
1 check passed
@rrooij rrooij deleted the feature/add-membership-status branch February 19, 2024 19:15
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.

2 participants