From 5053b8099835428055f567770ef21ca6b9ab49be Mon Sep 17 00:00:00 2001 From: Markus Graf Date: Thu, 4 Dec 2025 12:15:55 +0100 Subject: [PATCH] feat: Add organization member support with memberType field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements support for both individual persons and organizations as members. Changes: - Added MemberType enum (INDIVIDUAL, ORGANIZATION) to distinguish member types - Added member_type column to database (defaults to INDIVIDUAL for backward compatibility) - Added company_name field for organizations - Made first_name nullable (required for individuals, optional for organizations) - Implemented conditional validation: - INDIVIDUAL members require first_name - ORGANIZATION members require company_name - Updated GraphQL schema with new memberType and companyName fields - Updated all resolvers to handle new fields and validation - Added comprehensive unit tests for validation logic - Updated existing tests to work with new fields - All 53 tests passing Technical notes: - Using memberType instead of 'kind' to avoid GraphQL introspection conflicts - Using native_enum=False for SQLite compatibility - Using batch_alter_table for SQLite ALTER COLUMN compatibility - Backward compatible: existing members automatically become INDIVIDUAL type 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- ...b2d83cc_add_organization_member_support.py | 57 ++++++++ .../support-organization-members/design.md | 31 +++++ .../support-organization-members/proposal.md | 19 +++ .../specs/organization-support/spec.md | 59 +++++++++ .../support-organization-members/tasks.md | 10 ++ src/models/member.py | 30 ++++- src/resolvers/member.py | 40 +++++- src/schemas/member.py | 20 ++- src/validation.py | 18 ++- tests/integration/test_member_model.py | 4 +- tests/integration/test_member_mutations.py | 2 +- tests/unit/test_validation.py | 125 +++++++++++++++--- 12 files changed, 375 insertions(+), 40 deletions(-) create mode 100644 migrations/versions/b61b6b2d83cc_add_organization_member_support.py create mode 100644 openspec/changes/support-organization-members/design.md create mode 100644 openspec/changes/support-organization-members/proposal.md create mode 100644 openspec/changes/support-organization-members/specs/organization-support/spec.md create mode 100644 openspec/changes/support-organization-members/tasks.md diff --git a/migrations/versions/b61b6b2d83cc_add_organization_member_support.py b/migrations/versions/b61b6b2d83cc_add_organization_member_support.py new file mode 100644 index 0000000..06ec660 --- /dev/null +++ b/migrations/versions/b61b6b2d83cc_add_organization_member_support.py @@ -0,0 +1,57 @@ +"""add organization member support + +Revision ID: b61b6b2d83cc +Revises: 0f79356844b5 +Create Date: 2025-12-04 12:03:16.548989 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'b61b6b2d83cc' +down_revision: Union[str, Sequence[str], None] = '0f79356844b5' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # Add member_type column with default 'INDIVIDUAL' + op.add_column( + 'members', + sa.Column('member_type', sa.String(length=20), nullable=False, server_default='INDIVIDUAL') + ) + + # Add company_name column (nullable) + op.add_column( + 'members', + sa.Column('company_name', sa.String(length=200), nullable=True) + ) + + # Alter first_name to be nullable (SQLite compatible using batch operations) + with op.batch_alter_table('members', schema=None) as batch_op: + batch_op.alter_column( + 'first_name', + existing_type=sa.String(length=100), + nullable=True + ) + + +def downgrade() -> None: + """Downgrade schema.""" + # Remove company_name and member_type columns + op.drop_column('members', 'company_name') + op.drop_column('members', 'member_type') + + # Revert first_name to nullable=False + # Note: This may fail if there are organization members without first_name + with op.batch_alter_table('members', schema=None) as batch_op: + batch_op.alter_column( + 'first_name', + existing_type=sa.String(length=100), + nullable=False + ) diff --git a/openspec/changes/support-organization-members/design.md b/openspec/changes/support-organization-members/design.md new file mode 100644 index 0000000..b2702b4 --- /dev/null +++ b/openspec/changes/support-organization-members/design.md @@ -0,0 +1,31 @@ +# Design: Organization Member Support + +## Data Model Changes + +### Member Table +- Add `member_type` column: Enum (`INDIVIDUAL`, `ORGANIZATION`). Default to `INDIVIDUAL` for existing records. +- Add `company_name` column: String, nullable. +- Alter `first_name`: Change from `nullable=False` to `nullable=True`. + +### Constraints +- **Application Level**: + - If `member_type` is `INDIVIDUAL`: `first_name` must be present. + - If `member_type` is `ORGANIZATION`: `company_name` must be present. +- **Database Level**: + - We can't easily enforce conditional not-null constraints in standard SQL without triggers or check constraints. For simplicity and portability (SQLite/Postgres), we will enforce this at the application level (Pydantic/SQLAlchemy validators) and potentially add a CHECK constraint if supported by both dialects easily. + - `first_name` will become nullable in the DB schema. + +## API Changes + +### GraphQL Schema +- Update `Member` type to include `memberType` and `companyName`. +- Update `createMember` and `updateMember` inputs. +- Add validation logic in resolvers to enforce the conditional requirements. + +## Migration Strategy +1. Add `member_type` column with default 'INDIVIDUAL'. +2. Add `company_name` column. +3. Alter `first_name` to be nullable. + +## Technical Note +- Using `memberType` instead of `kind` to avoid conflicts with GraphQL introspection queries which use `kind` as a metadata field. diff --git a/openspec/changes/support-organization-members/proposal.md b/openspec/changes/support-organization-members/proposal.md new file mode 100644 index 0000000..86ce51f --- /dev/null +++ b/openspec/changes/support-organization-members/proposal.md @@ -0,0 +1,19 @@ +# Proposal: Support Organization Members + +## Summary +Update the member data model to support both individual persons and organizations. Organizations will have a company name instead of a mandatory first name. + +## Background +Currently, the system only supports individual members with a mandatory first name. We need to accommodate organizational members (companies, associations, etc.) where the primary identifier is the organization name. + +## Goals +- Allow distinguishing between individual and organization members. +- Make `first_name` optional for organizations. +- Add `company_name` field, mandatory for organizations. +- Ensure data integrity based on member type. + +## Non-Goals +- Complex hierarchy of organizations and contacts (for now, just a simple distinction). + +## Technical Note +- Field name: `memberType` (not `kind` to avoid GraphQL introspection conflicts) diff --git a/openspec/changes/support-organization-members/specs/organization-support/spec.md b/openspec/changes/support-organization-members/specs/organization-support/spec.md new file mode 100644 index 0000000..0cc0e68 --- /dev/null +++ b/openspec/changes/support-organization-members/specs/organization-support/spec.md @@ -0,0 +1,59 @@ +# Spec: Organization Support + +## ADDED Requirements + +### Requirement: Distinguish member kind + +The system MUST allow defining a member as either an individual or an organization. + +#### Scenario: Create individual member +**Given** client provides kind="INDIVIDUAL" +**And** first_name="John" +**When** createMember mutation executes +**Then** member is created with kind INDIVIDUAL +**And** first_name is "John" + +#### Scenario: Create organization member +**Given** client provides kind="ORGANIZATION" +**And** company_name="Acme Corp" +**When** createMember mutation executes +**Then** member is created with kind ORGANIZATION +**And** company_name is "Acme Corp" + +### Requirement: Conditional mandatory fields + +The system MUST enforce different mandatory fields based on member kind. + +#### Scenario: Organization requires company name +**Given** client provides kind="ORGANIZATION" +**And** company_name is missing or empty +**When** createMember mutation executes +**Then** validation error is raised +**And** error message indicates company_name is required for organizations + +#### Scenario: Organization allows missing first name +**Given** client provides kind="ORGANIZATION" +**And** company_name="Acme Corp" +**And** first_name is missing +**When** createMember mutation executes +**Then** member is created successfully +**And** first_name is null + +#### Scenario: Individual requires first name +**Given** client provides kind="INDIVIDUAL" +**And** first_name is missing +**When** createMember mutation executes +**Then** validation error is raised +**And** error message indicates first_name is required for individuals + +## MODIFIED Requirements + +### Requirement: Member data structure + +The Member data structure MUST be modified to include kind and company_name, and allow nullable first_name. + +#### Scenario: Query member includes new fields +**Given** member exists with kind="ORGANIZATION" and company_name="Acme Corp" +**When** member query executes requesting kind and companyName +**Then** kind is returned as "ORGANIZATION" +**And** companyName is returned as "Acme Corp" diff --git a/openspec/changes/support-organization-members/tasks.md b/openspec/changes/support-organization-members/tasks.md new file mode 100644 index 0000000..33aa4a2 --- /dev/null +++ b/openspec/changes/support-organization-members/tasks.md @@ -0,0 +1,10 @@ +# Tasks: Support Organization Members + +- [x] Create database migration to add `member_type` and `company_name` and make `first_name` nullable +- [x] Update SQLAlchemy `Member` model with new fields and validation logic +- [x] Update Pydantic schemas / GraphQL inputs to support new fields (`memberType`, `companyName`) +- [x] Update `createMember` and `updateMember` resolvers with conditional validation +- [x] Update tests to cover new scenarios (organization creation, validation rules) +- [x] Verify backward compatibility for existing individual members + +**Note**: Using `memberType` field name (not `kind`) to avoid GraphQL introspection conflicts. diff --git a/src/models/member.py b/src/models/member.py index 71ee5fa..4e91e7d 100644 --- a/src/models/member.py +++ b/src/models/member.py @@ -1,12 +1,19 @@ +import enum from datetime import datetime from typing import Optional -from sqlalchemy import String, func +from sqlalchemy import Enum, String, func from sqlalchemy.orm import Mapped, mapped_column from src.database import Base +class MemberType(str, enum.Enum): + """Member type enumeration.""" + INDIVIDUAL = "INDIVIDUAL" + ORGANIZATION = "ORGANIZATION" + + class Member(Base): """Member model for club/society management.""" @@ -15,8 +22,19 @@ class Member(Base): # Primary key id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) - # Required field - first_name: Mapped[str] = mapped_column(String(100), nullable=False) + # Member type (default to INDIVIDUAL for backward compatibility) + member_type: Mapped[MemberType] = mapped_column( + Enum(MemberType, native_enum=False, length=20), + nullable=False, + default=MemberType.INDIVIDUAL, + server_default="INDIVIDUAL" + ) + + # First name (required for individuals, optional for organizations) + first_name: Mapped[Optional[str]] = mapped_column(String(100), nullable=True) + + # Organization name (required for organizations) + company_name: Mapped[Optional[str]] = mapped_column(String(200), nullable=True) # Optional fields last_name: Mapped[Optional[str]] = mapped_column(String(100), nullable=True) @@ -40,4 +58,8 @@ class Member(Base): ) def __repr__(self) -> str: - return f"" + if self.member_type == MemberType.ORGANIZATION: + name = self.company_name + else: + name = f"{self.first_name} {self.last_name or ''}".strip() + return f"" diff --git a/src/resolvers/member.py b/src/resolvers/member.py index 83af95e..e92bf0b 100644 --- a/src/resolvers/member.py +++ b/src/resolvers/member.py @@ -6,7 +6,7 @@ from sqlalchemy import select from src.database import async_session_maker from src.models.member import Member as MemberModel from src.schemas.member import CreateMemberInput, Member, UpdateMemberInput -from src.validation import validate_email, validate_first_name, validate_phone +from src.validation import validate_email, validate_phone, validate_member_type_requirements class MemberNotFoundError(Exception): @@ -31,7 +31,9 @@ class Query: return Member( id=db_member.id, + member_type=db_member.member_type, first_name=db_member.first_name, + company_name=db_member.company_name, last_name=db_member.last_name, street=db_member.street, apartment_number=db_member.apartment_number, @@ -58,7 +60,9 @@ class Query: return [ Member( id=m.id, + member_type=m.member_type, first_name=m.first_name, + company_name=m.company_name, last_name=m.last_name, street=m.street, apartment_number=m.apartment_number, @@ -79,8 +83,12 @@ class Mutation: @strawberry.mutation async def create_member(self, input: CreateMemberInput) -> Member: """Create a new member with validation.""" - # Validate required field - validate_first_name(input.first_name) + # Validate conditional requirements based on member type + validate_member_type_requirements( + member_type=input.member_type.value, + first_name=input.first_name, + company_name=input.company_name + ) # Conditional validation (only if fields are provided) validate_email(input.email) @@ -89,7 +97,9 @@ class Mutation: async with async_session_maker() as session: # Create new member db_member = MemberModel( + member_type=input.member_type, first_name=input.first_name, + company_name=input.company_name, last_name=input.last_name, street=input.street, apartment_number=input.apartment_number, @@ -106,7 +116,9 @@ class Mutation: return Member( id=db_member.id, + member_type=db_member.member_type, first_name=db_member.first_name, + company_name=db_member.company_name, last_name=db_member.last_name, street=db_member.street, apartment_number=db_member.apartment_number, @@ -132,15 +144,29 @@ class Mutation: if db_member is None: raise MemberNotFoundError(f"Member with ID {input.id} not found") - # Validate if fields are being updated - if input.first_name is not None: - validate_first_name(input.first_name) + # Determine final values for validation (combining existing and new values) + final_member_type = input.member_type.value if input.member_type is not None else db_member.member_type.value + final_first_name = input.first_name if input.first_name is not None else db_member.first_name + final_company_name = input.company_name if input.company_name is not None else db_member.company_name + + # Validate conditional requirements with final values + validate_member_type_requirements( + member_type=final_member_type, + first_name=final_first_name, + company_name=final_company_name + ) + + # Conditional validation (only if fields are provided) validate_email(input.email) validate_phone(input.phone) # Update fields if provided + if input.member_type is not None: + db_member.member_type = input.member_type if input.first_name is not None: db_member.first_name = input.first_name + if input.company_name is not None: + db_member.company_name = input.company_name if input.last_name is not None: db_member.last_name = input.last_name if input.street is not None: @@ -163,7 +189,9 @@ class Mutation: return Member( id=db_member.id, + member_type=db_member.member_type, first_name=db_member.first_name, + company_name=db_member.company_name, last_name=db_member.last_name, street=db_member.street, apartment_number=db_member.apartment_number, diff --git a/src/schemas/member.py b/src/schemas/member.py index e980eeb..f2bee4d 100644 --- a/src/schemas/member.py +++ b/src/schemas/member.py @@ -1,15 +1,25 @@ from datetime import datetime +from enum import Enum from typing import Optional import strawberry +@strawberry.enum +class MemberType(str, Enum): + """Member type enumeration.""" + INDIVIDUAL = "INDIVIDUAL" + ORGANIZATION = "ORGANIZATION" + + @strawberry.type class Member: """GraphQL Member type.""" id: int - first_name: str + member_type: MemberType + first_name: Optional[str] + company_name: Optional[str] last_name: Optional[str] street: Optional[str] apartment_number: Optional[str] @@ -24,9 +34,11 @@ class Member: @strawberry.input class CreateMemberInput: - """Input type for creating a member - only firstName required.""" + """Input type for creating a member - conditional requirements based on memberType.""" - first_name: str + member_type: MemberType = MemberType.INDIVIDUAL + first_name: Optional[str] = None + company_name: Optional[str] = None last_name: Optional[str] = None street: Optional[str] = None apartment_number: Optional[str] = None @@ -42,7 +54,9 @@ class UpdateMemberInput: """Input type for updating a member - all fields optional except id.""" id: int + member_type: Optional[MemberType] = None first_name: Optional[str] = None + company_name: Optional[str] = None last_name: Optional[str] = None street: Optional[str] = None apartment_number: Optional[str] = None diff --git a/src/validation.py b/src/validation.py index 3dd8f6f..3c318d3 100644 --- a/src/validation.py +++ b/src/validation.py @@ -13,10 +13,20 @@ EMAIL_PATTERN = re.compile(r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$") PHONE_PATTERN = re.compile(r"^\+?[1-9]\d{1,14}$") # E.164 format -def validate_first_name(first_name: str) -> None: - """Ensure firstName is provided and non-empty.""" - if not first_name or first_name.strip() == "": - raise ValidationError("firstName cannot be empty") +def validate_member_type_requirements( + member_type: str, + first_name: Optional[str], + company_name: Optional[str] +) -> None: + """Validate conditional requirements based on member type.""" + if member_type == "INDIVIDUAL": + if not first_name or first_name.strip() == "": + raise ValidationError("firstName is required for individual members") + elif member_type == "ORGANIZATION": + if not company_name or company_name.strip() == "": + raise ValidationError("companyName is required for organization members") + else: + raise ValidationError(f"Invalid member type: {member_type}. Must be INDIVIDUAL or ORGANIZATION") def validate_email(email: Optional[str]) -> None: diff --git a/tests/integration/test_member_model.py b/tests/integration/test_member_model.py index bc39681..1c22a83 100644 --- a/tests/integration/test_member_model.py +++ b/tests/integration/test_member_model.py @@ -80,5 +80,5 @@ class TestMemberModel: repr_str = repr(member) assert "Member" in repr_str assert f"id={member.id}" in repr_str - assert "first_name='David'" in repr_str - assert "last_name='Smith'" in repr_str + assert "type=INDIVIDUAL" in repr_str + assert "name='David Smith'" in repr_str diff --git a/tests/integration/test_member_mutations.py b/tests/integration/test_member_mutations.py index 7e170d6..527883e 100644 --- a/tests/integration/test_member_mutations.py +++ b/tests/integration/test_member_mutations.py @@ -86,7 +86,7 @@ class TestCreateMemberMutation: mutation = Mutation() input_data = CreateMemberInput(first_name="") - with pytest.raises(ValidationError, match="firstName cannot be empty"): + with pytest.raises(ValidationError, match="firstName is required for individual members"): await mutation.create_member(input=input_data) diff --git a/tests/unit/test_validation.py b/tests/unit/test_validation.py index 800e88a..481c4b2 100644 --- a/tests/unit/test_validation.py +++ b/tests/unit/test_validation.py @@ -2,26 +2,12 @@ import pytest -from src.validation import ValidationError, validate_email, validate_first_name, validate_phone - - -class TestFirstNameValidation: - """Tests for validate_first_name function.""" - - def test_valid_first_name(self): - """Test that valid firstName passes validation.""" - validate_first_name("Alice") - # No exception = pass - - def test_empty_string_raises_error(self): - """Test that empty string raises ValidationError.""" - with pytest.raises(ValidationError, match="firstName cannot be empty"): - validate_first_name("") - - def test_whitespace_only_raises_error(self): - """Test that whitespace-only string raises ValidationError.""" - with pytest.raises(ValidationError, match="firstName cannot be empty"): - validate_first_name(" ") +from src.validation import ( + ValidationError, + validate_email, + validate_phone, + validate_member_type_requirements, +) class TestEmailValidation: @@ -110,3 +96,102 @@ class TestPhoneValidation: """Test that empty string passes validation (optional field).""" validate_phone("") # No exception = pass + + +class TestMemberTypeRequirements: + """Tests for validate_member_type_requirements function.""" + + def test_individual_with_first_name_valid(self): + """Test INDIVIDUAL with firstName passes.""" + validate_member_type_requirements( + member_type="INDIVIDUAL", + first_name="John", + company_name=None + ) + + def test_individual_without_first_name_invalid(self): + """Test INDIVIDUAL without firstName fails.""" + with pytest.raises(ValidationError, match="firstName is required for individual members"): + validate_member_type_requirements( + member_type="INDIVIDUAL", + first_name=None, + company_name=None + ) + + def test_individual_with_empty_first_name_invalid(self): + """Test INDIVIDUAL with empty firstName fails.""" + with pytest.raises(ValidationError, match="firstName is required for individual members"): + validate_member_type_requirements( + member_type="INDIVIDUAL", + first_name="", + company_name=None + ) + + def test_individual_with_whitespace_first_name_invalid(self): + """Test INDIVIDUAL with whitespace-only firstName fails.""" + with pytest.raises(ValidationError, match="firstName is required for individual members"): + validate_member_type_requirements( + member_type="INDIVIDUAL", + first_name=" ", + company_name=None + ) + + def test_organization_with_company_name_valid(self): + """Test ORGANIZATION with companyName passes.""" + validate_member_type_requirements( + member_type="ORGANIZATION", + first_name=None, + company_name="Acme Corp" + ) + + def test_organization_without_company_name_invalid(self): + """Test ORGANIZATION without companyName fails.""" + with pytest.raises(ValidationError, match="companyName is required for organization members"): + validate_member_type_requirements( + member_type="ORGANIZATION", + first_name=None, + company_name=None + ) + + def test_organization_with_empty_company_name_invalid(self): + """Test ORGANIZATION with empty companyName fails.""" + with pytest.raises(ValidationError, match="companyName is required for organization members"): + validate_member_type_requirements( + member_type="ORGANIZATION", + first_name=None, + company_name="" + ) + + def test_organization_with_whitespace_company_name_invalid(self): + """Test ORGANIZATION with whitespace-only companyName fails.""" + with pytest.raises(ValidationError, match="companyName is required for organization members"): + validate_member_type_requirements( + member_type="ORGANIZATION", + first_name=None, + company_name=" " + ) + + def test_invalid_member_type_raises_error(self): + """Test invalid member type raises ValidationError.""" + with pytest.raises(ValidationError, match="Invalid member type"): + validate_member_type_requirements( + member_type="INVALID", + first_name="John", + company_name=None + ) + + def test_organization_can_have_first_name_optional(self): + """Test ORGANIZATION can optionally have firstName.""" + validate_member_type_requirements( + member_type="ORGANIZATION", + first_name="John Doe", + company_name="Acme Corp" + ) + + def test_individual_can_have_company_name_optional(self): + """Test INDIVIDUAL can optionally have companyName (edge case).""" + validate_member_type_requirements( + member_type="INDIVIDUAL", + first_name="John", + company_name="Acme Corp" + )