From abfccf96e8a17cfc77eda28a855b8196324cdaa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Fri, 13 Mar 2026 12:29:25 +0100 Subject: [PATCH 01/31] Upgrade to ruby 3.3.x --- .ruby-version | 2 +- .tool-versions | 2 +- Gemfile | 2 +- Gemfile.lock | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.ruby-version b/.ruby-version index be94e6f5..eb39e538 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.2.2 +3.3 diff --git a/.tool-versions b/.tool-versions index 124d9987..bf90baeb 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ -ruby 3.2.2 +ruby 3.3 nodejs 21.5.0 yarn 1.22.19 diff --git a/Gemfile b/Gemfile index 541825c1..1feb8ae8 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ source "https://rubygems.org" git_source(:github) { |repo| "https://github.com/#{repo}.git" } -ruby ">= 3.2.2" +ruby "~> 3.3" # Bundle edge Rails instead: gem "rails", github: "rails/rails", branch: "main" gem "rails", "~> 7.1", ">= 7.0.4.2" diff --git a/Gemfile.lock b/Gemfile.lock index 4d3f1caf..d85d9306 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -472,7 +472,7 @@ DEPENDENCIES wicked (~> 2.0) RUBY VERSION - ruby 3.2.2p53 + ruby 3.3.4p94 BUNDLED WITH 2.4.22 From dd79562825a603cdeb3b9236c9b2d0844d29da20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 13:04:43 +0100 Subject: [PATCH 02/31] Add shared projects design spec Co-Authored-By: Claude Opus 4.6 (1M context) --- .../2026-03-17-shared-projects-design.md | 231 ++++++++++++++++++ 1 file changed, 231 insertions(+) create mode 100644 docs/superpowers/specs/2026-03-17-shared-projects-design.md diff --git a/docs/superpowers/specs/2026-03-17-shared-projects-design.md b/docs/superpowers/specs/2026-03-17-shared-projects-design.md new file mode 100644 index 00000000..c53963cb --- /dev/null +++ b/docs/superpowers/specs/2026-03-17-shared-projects-design.md @@ -0,0 +1,231 @@ +# Shared Projects Design Spec + +## Overview + +Allow organizations to share projects with other organizations. An admin from the owning organization invites external users by email. When a user from another organization accepts, an org-to-org relationship is automatically established. The owning org retains full control; guest org admins get read access to the project and all time entries; each org maintains independent rates. + +## Requirements + +1. **Org-to-org sharing**: Admin from org_one shares a project with org_two by inviting org_two users via email. The org-to-org link is created implicitly on first invitation acceptance. +2. **User selection**: Org_one admin selects specific org_two users (by email invitation, like today). +3. **Guest admin visibility**: Org_two admins can read ALL time_regs on the shared project (including org_one users' entries). +4. **Independent rates**: Each org sets their own project-level and task-level rates in their own currency. +5. **Shared task list**: Tasks are defined by org_one. Each org sets their own rate per task. +6. **Multiple guest orgs**: A project can be shared with any number of organizations. +7. **Disconnection**: Either side can disconnect. Time_regs are preserved. + +## Data Model + +### New Tables + +#### `project_shares` + +| Column | Type | Constraints | +|--------|------|------------| +| id | bigint | PK | +| project_id | bigint | NOT NULL, FK → projects | +| organization_id | bigint | NOT NULL, FK → organizations | +| rate | integer | DEFAULT 0 | +| created_at | datetime | NOT NULL | +| updated_at | datetime | NOT NULL | + +- Unique index on `[project_id, organization_id]` +- `organization_id` refers to the **guest** organization (not the project owner) + +#### `project_share_task_rates` + +| Column | Type | Constraints | +|--------|------|------------| +| id | bigint | PK | +| project_share_id | bigint | NOT NULL, FK → project_shares | +| assigned_task_id | bigint | NOT NULL, FK → assigned_tasks | +| rate | integer | DEFAULT 0 | +| created_at | datetime | NOT NULL | +| updated_at | datetime | NOT NULL | + +- Unique index on `[project_share_id, assigned_task_id]` + +### New Models + +#### `ProjectShare` + +```ruby +class ProjectShare < ApplicationRecord + include RateConvertible + + belongs_to :project + belongs_to :organization + + has_many :project_share_task_rates, dependent: :destroy + + validates :organization_id, uniqueness: { scope: :project_id } + validate :organization_is_not_project_owner + + private + + def organization_is_not_project_owner + errors.add(:organization, :is_project_owner) if organization_id == project&.organization&.id + end +end +``` + +#### `ProjectShareTaskRate` + +```ruby +class ProjectShareTaskRate < ApplicationRecord + include RateConvertible + + belongs_to :project_share + belongs_to :assigned_task + + validates :assigned_task_id, uniqueness: { scope: :project_share_id } +end +``` + +### Existing Model Changes + +**Project:** +- `has_many :project_shares, dependent: :destroy` +- `has_many :shared_organizations, through: :project_shares, source: :organization` + +**Organization:** +- `has_many :project_shares, dependent: :destroy` +- `has_many :shared_projects, through: :project_shares, source: :project` + +## Invitation & Sharing Flow + +### Establishing the Org-to-Org Link + +The existing `ProjectInvitationService` and `ProjectInvitation#accept!` flow is extended: + +1. Org_one admin invites an external user by email (existing flow) +2. User accepts invitation (existing: creates `AccessInfo` + `ProjectAccess`) +3. **New:** If the user's organization differs from the project's organization, `find_or_create` a `ProjectShare` between the project and the user's organization +4. The `ProjectShare` is created with `rate: 0` (guest org admin sets rates later) + +Subsequent invitations to users from the same org reuse the existing `ProjectShare`. + +### Disconnecting + +**Org_one disconnects org_two:** +1. Destroy the `ProjectShare` (cascades to `ProjectShareTaskRate` records) +2. Destroy all `ProjectAccess` records for org_two users on this project +3. Cancel pending `ProjectInvitation` records for org_two users +4. Time_regs are preserved (they reference `assigned_task`, not `project_access`) + +**Org_two disconnects from project:** +- Same as above — destroys their `ProjectShare` and their users' `ProjectAccess` records +- Time_regs preserved + +## Authorization & Scoping + +### Guest Org Admin Permissions + +When a `ProjectShare` exists between a project and org_two, org_two admins can: +- View the project (name, description, tasks) +- View ALL time_regs on the project (from any org) +- Manage their own org's rates (`ProjectShare` rate + `ProjectShareTaskRate` rates) +- Disconnect their org from the project + +They cannot: +- Edit the project (name, description, tasks, billable status) +- Create/edit/delete time_regs for other orgs' users +- See other orgs' rates +- Manage project access (invitations are org_one admin's job) + +### Guest Org Member Permissions + +Org_two members with `ProjectAccess`: +- Can create/edit/delete their own time_regs +- Can view project details and task list +- Cannot see rates +- Cannot see other users' time_regs (unless they are admin) + +### Policy Changes + +**ProjectPolicy scope (`:own`):** + +```ruby +scope_for :relation, :own do |relation| + # Org's own projects + own = relation.joins(client: :organization) + .where(organizations: { id: organization.id }) + + # Shared projects via ProjectShare + shared = relation.joins(:project_shares) + .where(project_shares: { organization_id: organization.id }) + + combined = own.or(shared) + + unless user.organization_admin? + combined = combined.joins(:project_accesses) + .where(project_accesses: user.project_accesses) + end + combined.distinct +end +``` + +Org_two admins see shared projects automatically via `ProjectShare` (no individual `ProjectAccess` needed). Org_two non-admin users still require individual `ProjectAccess`. + +**TimeRegPolicy:** + +Extended so org_two admins can read (not write) all time_regs on shared projects. Org_two members can only read/write their own time_regs. + +**New policy actions:** +- `project#manage_share?` — org_one admin only (disconnect guest org, manage sharing) +- `project#disconnect_share?` — org_two admin (disconnect own org) +- `project#view_shared?` — org_two admin read access + +## Rate Management + +### Rate Ownership + +| Who | Project rate | Task rates | +|-----|-------------|------------| +| Org_one (owner) | `project.rate` | `assigned_task.rate` | +| Org_two (guest) | `project_share.rate` | `project_share_task_rate.rate` | + +### Rate Resolution for Reports + +1. Determine which org is viewing the report +2. If the org owns the project: use `assigned_task.rate` falling back to `project.rate` (existing behavior) +3. If the org is a guest: use `project_share_task_rate.rate` falling back to `project_share.rate` +4. If the guest org hasn't set rates (rate = 0): show as unset/zero + +### Currency + +Each org has its own currency. Rates on `ProjectShare` and `ProjectShareTaskRate` are in the guest org's currency. No cross-currency conversion — each org sees their own rates in their own currency. + +### Rate Management UI + +Org_two admins see a "Rates" section when viewing a shared project, where they can: +- Set their org's project-level rate on the `ProjectShare` +- Set per-task rates via `ProjectShareTaskRate` for each `AssignedTask` + +This is separate from the project edit page (which org_two cannot access). + +## UI & Navigation + +### Org_one Admin (Project Owner) + +- Project show page gains a "Shared with" section listing guest organizations with a "Disconnect" action per org +- Existing "Invite external user" flow unchanged (email-based) +- Time_regs view unchanged + +### Org_two Admin (Guest Org) + +- Shared projects appear in the project list, visually distinguished with a badge/tag showing the owning org name +- Project show page is **read-only**: project details, task list, all time_regs +- A "Rates" tab/section for managing their org's rates +- A "Disconnect" action to leave the shared project + +### Org_two Member (Invited User) + +- Shared project appears in their project list (via `ProjectAccess`, same as today) +- Can create/edit/delete their own time_regs +- Sees tasks defined by org_one +- Does not see rates (same as regular non-admin members) + +### Navigation + +No new top-level navigation. Shared projects are mixed into the existing projects list with a visual indicator of ownership. From 3678a69407e52fa16ceed8b7fb8e64d9ed2f50ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 13:09:45 +0100 Subject: [PATCH 03/31] Update shared projects spec after review Address issues found during spec review: - Rate resolution is now org-context-aware (used_rate_for method) - TimeRegPolicy scope extended for shared project visibility - ProjectPolicy uses subquery approach to avoid JOIN incompatibility - Added ProjectSharePolicy and ProjectShareTaskRatePolicy - Post-disconnection time_reg visibility rules defined - AssignedTask rate-change archiving migrates ProjectShareTaskRate - Workspace::ProjectPolicy changes for guest admin read access - Task/Client scope adjustments documented - Controller structure and routes defined - API considerations noted - Edge cases documented (multi-org users, promotions, soft deletes) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../2026-03-17-shared-projects-design.md | 199 +++++++++++++++--- 1 file changed, 173 insertions(+), 26 deletions(-) diff --git a/docs/superpowers/specs/2026-03-17-shared-projects-design.md b/docs/superpowers/specs/2026-03-17-shared-projects-design.md index c53963cb..3da8d29c 100644 --- a/docs/superpowers/specs/2026-03-17-shared-projects-design.md +++ b/docs/superpowers/specs/2026-03-17-shared-projects-design.md @@ -100,8 +100,8 @@ The existing `ProjectInvitationService` and `ProjectInvitation#accept!` flow is 1. Org_one admin invites an external user by email (existing flow) 2. User accepts invitation (existing: creates `AccessInfo` + `ProjectAccess`) -3. **New:** If the user's organization differs from the project's organization, `find_or_create` a `ProjectShare` between the project and the user's organization -4. The `ProjectShare` is created with `rate: 0` (guest org admin sets rates later) +3. **New:** The `ProjectShare` is created for the organization passed to `accept!(organization)` — this is the org the user chose to accept into, not an implicit "user's default org." A user who belongs to multiple orgs selects which org context to accept under. +4. `ProjectShare.find_or_create_by!(project: project, organization: organization)` with `rate: 0` Subsequent invitations to users from the same org reuse the existing `ProjectShare`. @@ -110,13 +110,29 @@ Subsequent invitations to users from the same org reuse the existing `ProjectSha **Org_one disconnects org_two:** 1. Destroy the `ProjectShare` (cascades to `ProjectShareTaskRate` records) 2. Destroy all `ProjectAccess` records for org_two users on this project -3. Cancel pending `ProjectInvitation` records for org_two users +3. Cancel pending `ProjectInvitation` records where `invited_email` matches existing org_two users (by email lookup against org_two's `access_infos → users`). Invitations to not-yet-registered users cannot be reliably matched — they remain pending and will fail validation if accepted after disconnection (since `ProjectAccess` creation would succeed but the `ProjectShare` would be absent). 4. Time_regs are preserved (they reference `assigned_task`, not `project_access`) **Org_two disconnects from project:** - Same as above — destroys their `ProjectShare` and their users' `ProjectAccess` records - Time_regs preserved +### Post-Disconnection Time_Reg Visibility + +After disconnection, time_regs from org_two users remain in the database linked to `assigned_task → project → client → org_one`. These time_regs are: +- **Visible to org_one admins**: They own the project and see all time_regs on it (existing behavior via `TimeRegPolicy` scoping through project organization). +- **Invisible to org_two**: The `ProjectShare` no longer exists, so org_two admins lose read access. Org_two members lose `ProjectAccess`, so they also lose visibility. This is intentional — disconnection severs the relationship. +- **Org_two users' own time_regs**: For personal history purposes, org_two users can still see their own time_regs via the `user_id` scope (the existing `TimeRegPolicy` allows users to see their own entries regardless of project ownership). + +### Soft Delete Interaction + +When org_one soft-deletes (discards) a shared project: +- The project's `default_scope -> { kept }` hides it from all queries +- `ProjectShare` records remain in the database but are effectively orphaned +- No automatic disconnection is triggered — the shares are inert while the project is discarded +- If the project is later restored (undiscarded), the shares resume functioning +- If the project is permanently destroyed, `dependent: :destroy` on `has_many :project_shares` cascades the deletion + ## Authorization & Scoping ### Guest Org Admin Permissions @@ -124,6 +140,7 @@ Subsequent invitations to users from the same org reuse the existing `ProjectSha When a `ProjectShare` exists between a project and org_two, org_two admins can: - View the project (name, description, tasks) - View ALL time_regs on the project (from any org) +- View org_one user names on time_regs (first_name, last_name only — no email or other details) - Manage their own org's rates (`ProjectShare` rate + `ProjectShareTaskRate` rates) - Disconnect their org from the project @@ -141,25 +158,29 @@ Org_two members with `ProjectAccess`: - Cannot see rates - Cannot see other users' time_regs (unless they are admin) +### Guest Org Spectator Permissions + +Org_two spectators follow the same rules as regular spectators: read-only access to projects they can see. If a spectator has `ProjectAccess` to a shared project, they can view the project and time_regs on it (scoped the same as today's spectator behavior). They cannot log time. + ### Policy Changes -**ProjectPolicy scope (`:own`):** +**ProjectPolicy scope (`:own`) — uses subquery approach to avoid JOIN incompatibility:** ```ruby scope_for :relation, :own do |relation| - # Org's own projects - own = relation.joins(client: :organization) - .where(organizations: { id: organization.id }) + own_ids = relation.joins(client: :organization) + .where(organizations: { id: organization.id }) + .select(:id) - # Shared projects via ProjectShare - shared = relation.joins(:project_shares) - .where(project_shares: { organization_id: organization.id }) + shared_ids = relation.joins(:project_shares) + .where(project_shares: { organization_id: organization.id }) + .select(:id) - combined = own.or(shared) + combined = relation.where(id: own_ids).or(relation.where(id: shared_ids)) unless user.organization_admin? - combined = combined.joins(:project_accesses) - .where(project_accesses: user.project_accesses) + combined = combined.where(id: ProjectAccess.where(access_info: user.access_info(organization)) + .select(:project_id)) end combined.distinct end @@ -167,14 +188,51 @@ end Org_two admins see shared projects automatically via `ProjectShare` (no individual `ProjectAccess` needed). Org_two non-admin users still require individual `ProjectAccess`. -**TimeRegPolicy:** +**TimeRegPolicy scope — extended for shared project visibility:** + +```ruby +# For admins: own org's time_regs + time_regs on projects shared with this org +scope_for :relation, :own do |relation| + own = relation.joins(:organization).where(organizations: { id: organization.id }) + + shared_project_ids = ProjectShare.where(organization_id: organization.id).select(:project_id) + on_shared = relation.joins(assigned_task: :project) + .where(projects: { id: shared_project_ids }) + + if user.organization_admin? + own.or(on_shared).distinct + else + # Non-admin: own time_regs only (on both owned and shared projects) + relation.where(user: user) + end +end +``` + +**Workspace::ProjectPolicy — extended for guest admin read access:** + +The workspace project policy must allow guest org admins to view (but not edit) shared projects. Add a `show?` override that permits access when a `ProjectShare` exists for the user's current organization. `edit?`, `update?`, `destroy?` remain restricted to the owning org's admins. + +**New policies:** + +`ProjectSharePolicy`: +- `show?` — org_one admin or org_two admin (either side can view the share) +- `update?` — org_two admin only (manage their rates) +- `destroy?` — org_one admin or org_two admin (either side can disconnect) +- Scope: projects shared with the current organization + +`ProjectShareTaskRatePolicy`: +- `create?`, `update?`, `destroy?` — org_two admin only (manage their task rates) +- Scope: task rates belonging to the current org's project shares + +**New policy actions on ProjectPolicy:** +- `manage_share?` — org_one admin only (view shared orgs, disconnect guest org) +- `disconnect_share?` — org_two admin (disconnect own org) + +### Task & Client Scope Adjustments -Extended so org_two admins can read (not write) all time_regs on shared projects. Org_two members can only read/write their own time_regs. +**TaskPolicy scope:** Extended to include org_one's tasks when the user is on a shared project. When building task lists for time_reg forms on shared projects, include tasks from the project's owning org (via `assigned_tasks` on the project). -**New policy actions:** -- `project#manage_share?` — org_one admin only (disconnect guest org, manage sharing) -- `project#disconnect_share?` — org_two admin (disconnect own org) -- `project#view_shared?` — org_two admin read access +**ClientPolicy scope:** Not extended — guest orgs do not browse org_one's client list. Shared projects appear directly in the project list. Report filters for guest orgs show the shared project directly without the client hierarchy. ## Rate Management @@ -185,16 +243,50 @@ Extended so org_two admins can read (not write) all time_regs on shared projects | Org_one (owner) | `project.rate` | `assigned_task.rate` | | Org_two (guest) | `project_share.rate` | `project_share_task_rate.rate` | -### Rate Resolution for Reports +### Rate Resolution + +Rate resolution is context-dependent — it requires knowing which organization is viewing. + +**`TimeReg#used_rate` refactoring:** + +The existing `used_rate` method returns the owning org's rate. For shared projects, introduce a context-aware method: + +```ruby +# New method that accepts an organization for rate context +def used_rate_for(organization) + project_share = project.project_shares.find_by(organization: organization) + if project_share + task_rate = project_share.project_share_task_rates.find_by(assigned_task: assigned_task) + rate = task_rate&.rate || 0 + rate.positive? ? rate : project_share.rate + else + # Owning org — existing behavior + assigned_task.rate.positive? ? assigned_task.rate : project.rate + end +end + +# Keep existing method for backward compatibility +def used_rate + assigned_task.rate.positive? ? assigned_task.rate : project.rate +end +``` + +Reports and billing call `used_rate_for(current_organization)` instead of `used_rate`. The existing `used_rate` continues to work for contexts where only the owning org's rate matters. + +### AssignedTask Rate-Change Archiving + +When `AssignedTask#handle_rate_change` archives an old record and creates a new one, `ProjectShareTaskRate` records referencing the old `assigned_task_id` must be migrated: + +```ruby +# In handle_rate_change, after creating new_assigned_task: +ProjectShareTaskRate.where(assigned_task: self).update_all(assigned_task_id: new_assigned_task.id) +``` -1. Determine which org is viewing the report -2. If the org owns the project: use `assigned_task.rate` falling back to `project.rate` (existing behavior) -3. If the org is a guest: use `project_share_task_rate.rate` falling back to `project_share.rate` -4. If the guest org hasn't set rates (rate = 0): show as unset/zero +This preserves guest org task rates across rate changes. ### Currency -Each org has its own currency. Rates on `ProjectShare` and `ProjectShareTaskRate` are in the guest org's currency. No cross-currency conversion — each org sees their own rates in their own currency. +Each org has its own currency. Rates on `ProjectShare` and `ProjectShareTaskRate` are in the guest org's currency (inferred from `project_share.organization.currency`). No cross-currency conversion — each org sees their own rates in their own currency. ### Rate Management UI @@ -204,11 +296,54 @@ Org_two admins see a "Rates" section when viewing a shared project, where they c This is separate from the project edit page (which org_two cannot access). +## Controller Structure + +### New Controllers + +**`Workspace::ProjectSharesController`** — manages the org-to-org relationship: +- `index` — list guest orgs for a project (org_one admin) or list shared projects (org_two admin) +- `update` — org_two admin updates their rates on the `ProjectShare` +- `destroy` — either side disconnects + +**`Workspace::ProjectShareTaskRatesController`** — manages per-task rates for guest orgs: +- `create` / `update` — org_two admin sets task rates +- Nested under project_share routes + +### Existing Controller Changes + +**`Workspace::ProjectsController`:** +- `show` — detect if project is shared (guest org context), render read-only view +- Time_reg listing extended to show all entries on shared projects for guest admins + +### Routes + +```ruby +namespace :workspace do + resources :projects do + resources :project_shares, only: [:index, :destroy] do + resources :project_share_task_rates, only: [:create, :update] + member do + patch :update_rates # Bulk update project + task rates + end + end + end +end +``` + +## API Considerations + +The API layer (`Api::V1::*`) uses the same policies, so policy scope changes automatically apply. However: +- `Api::V1::ReportsController` does SQL-level aggregation — rate resolution must happen at the query level or post-processing, not just in Ruby +- API responses for shared projects should include a `shared: true` flag and `owner_organization` info +- API rate endpoints need to respect the same org-context rules + +These are implementation details to address during the API phase, not blockers for the initial implementation. + ## UI & Navigation ### Org_one Admin (Project Owner) -- Project show page gains a "Shared with" section listing guest organizations with a "Disconnect" action per org +- Project show page gains a "Shared with" section listing guest organizations (name only) with a "Disconnect" action per org - Existing "Invite external user" flow unchanged (email-based) - Time_regs view unchanged @@ -216,6 +351,7 @@ This is separate from the project edit page (which org_two cannot access). - Shared projects appear in the project list, visually distinguished with a badge/tag showing the owning org name - Project show page is **read-only**: project details, task list, all time_regs +- Time_regs show user names (first_name, last_name) for all orgs - A "Rates" tab/section for managing their org's rates - A "Disconnect" action to leave the shared project @@ -229,3 +365,14 @@ This is separate from the project edit page (which org_two cannot access). ### Navigation No new top-level navigation. Shared projects are mixed into the existing projects list with a visual indicator of ownership. + +## Edge Cases + +### Multi-org users +A user belonging to both org_one and org_two sees the project in both org contexts. When switching organizations, their permissions change accordingly (admin in org_one = full control, member in org_two = member-level access on the shared project). The `current_organization` determines which context applies. + +### User promoted to admin in guest org +If an org_two member with `ProjectAccess` is promoted to admin, they gain full guest-admin read access to the shared project. Their existing `ProjectAccess` record becomes redundant (admins see shared projects via `ProjectShare`) but is harmless to keep. + +### All guest org users removed but ProjectShare remains +If all org_two users' `ProjectAccess` records are individually removed (without disconnecting the org), the `ProjectShare` remains. Org_two admins still see the project. This is acceptable — the org-level share persists until explicitly disconnected. From ae9079d6fe31425d7f72c0a35e0b6e91bad82872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 13:15:13 +0100 Subject: [PATCH 04/31] Fix blocking issues in shared projects spec - Add concrete TimeRegPolicy action methods for shared projects (create?, show?, edit? etc. now handle cross-org access) - Add concrete TaskPolicy scope for shared projects (guest org users can see owning org's tasks via assigned_tasks) - Add export restriction note (no emails for cross-org users) - Add Reports::Summary/Result refactoring note - Fix route/controller inconsistency Co-Authored-By: Claude Opus 4.6 (1M context) --- .../2026-03-17-shared-projects-design.md | 120 +++++++++++++++--- 1 file changed, 105 insertions(+), 15 deletions(-) diff --git a/docs/superpowers/specs/2026-03-17-shared-projects-design.md b/docs/superpowers/specs/2026-03-17-shared-projects-design.md index 3da8d29c..2f90de80 100644 --- a/docs/superpowers/specs/2026-03-17-shared-projects-design.md +++ b/docs/superpowers/specs/2026-03-17-shared-projects-design.md @@ -188,26 +188,116 @@ end Org_two admins see shared projects automatically via `ProjectShare` (no individual `ProjectAccess` needed). Org_two non-admin users still require individual `ProjectAccess`. -**TimeRegPolicy scope — extended for shared project visibility:** +**TimeRegPolicy — action methods and scopes:** + +The existing action methods check `record.organization == user.current_organization`, which resolves through the project's owning org. For shared projects, this check fails for guest org users. Introduce a helper: ```ruby -# For admins: own org's time_regs + time_regs on projects shared with this org -scope_for :relation, :own do |relation| - own = relation.joins(:organization).where(organizations: { id: organization.id }) +class TimeRegPolicy < ApplicationPolicy + private + + def on_shared_project? + ProjectShare.exists?(project: record.project, organization: user.current_organization) + end + + def admin_of_shared_project? + user.organization_admin? && on_shared_project? + end + + public - shared_project_ids = ProjectShare.where(organization_id: organization.id).select(:project_id) - on_shared = relation.joins(assigned_task: :project) - .where(projects: { id: shared_project_ids }) + # show/edit/update/destroy — user can manage their own, or owning-org admin, + # or guest-org admin can READ (show only, not edit/update/destroy) + def show? + user == record.user || is_admin_allowed || admin_of_shared_project? + end + + %i[edit update destroy toggle_active].each do |action| + define_method("#{action}?") do + is_admin_allowed = user.organization_admin? && record.organization == user.current_organization + user == record.user || is_admin_allowed + end + end + def create? + return false if user.access_info.organization_spectator? + + same_organization = user.current_organization == record.organization + no_organization = record.organization.nil? + shared_project = on_shared_project? + + if user.organization_admin? + same_organization + else + # Allow if same org, no org (new record), or on a shared project (own entries only) + ((same_organization || no_organization || shared_project) && record.user == user) + end + end + + # Scope: admins see own org + shared project time_regs + # Members see only their own time_regs + scope_for :relation do |relation| + organization = user.current_organization + shared_project_ids = ProjectShare.where(organization_id: organization.id).select(:project_id) + + if user.organization_admin? + own = relation.joins(:organization).where(organizations: { id: organization.id }) + on_shared = relation.joins(assigned_task: :project).where(projects: { id: shared_project_ids }) + own.or(on_shared).distinct + elsif user.access_info.organization_spectator? + projects = authorized_scope(Project.all, type: :relation).all + relation.joins(:project).where(projects: projects).distinct + else + # Non-admin members: own time_regs only (covers both owned and shared projects) + relation.where(user: user).distinct + end + end +end +``` + +Key changes: +- `show?` allows guest org admins to **read** any time_reg on shared projects +- `edit?`/`update?`/`destroy?` unchanged — only own entries or owning-org admin +- `create?` allows guest org members to create their own time_regs on shared projects +- Default scope includes shared project time_regs for guest org admins +- Non-admin members use `user: user` scope which naturally includes their entries on shared projects + +**Export restrictions:** The `export` action must exclude email addresses for cross-org time_regs. When exporting time_regs from a shared project, only include `first_name` and `last_name` for users from other organizations — not their email. + +**TaskPolicy scope — extended for shared projects:** + +The existing scope filters tasks to the current organization. For shared projects, org_two users need to see org_one's tasks (which are assigned to the project). The scope is extended: + +```ruby +scope_for :relation do |relation| if user.organization_admin? + # Own org tasks + tasks assigned to shared projects + own = relation.where(organization: user.current_organization) + shared_project_ids = ProjectShare.where(organization_id: user.current_organization.id).select(:project_id) + on_shared = relation.joins(:assigned_tasks).where(assigned_tasks: { project_id: shared_project_ids }) own.or(on_shared).distinct + elsif user.access_info.organization_spectator? + projects = authorized_scope(Project.all, type: :relation).all + relation.joins(:projects).where(projects: projects).distinct else - # Non-admin: own time_regs only (on both owned and shared projects) - relation.where(user: user) + # Non-admin: own org tasks + tasks on shared projects they have access to + own = relation.joins(:users).where(organization: user.current_organization, users: { id: user.id }) + user_shared_project_ids = ProjectAccess.joins(:access_info) + .where(access_infos: { user_id: user.id, organization_id: user.current_organization.id }) + .joins(:project) + .merge(Project.joins(:project_shares).where(project_shares: { organization_id: user.current_organization.id })) + .select(:project_id) + on_shared = relation.joins(:assigned_tasks).where(assigned_tasks: { project_id: user_shared_project_ids }) + own.or(on_shared).distinct end end ``` +Key changes: +- Admin scope: includes tasks assigned to any project shared with their org +- Member scope: includes tasks assigned to shared projects they have `ProjectAccess` to +- Spectator scope: unchanged (derives from authorized project scope) + **Workspace::ProjectPolicy — extended for guest admin read access:** The workspace project policy must allow guest org admins to view (but not edit) shared projects. Add a `show?` override that permits access when a `ProjectShare` exists for the user's current organization. `edit?`, `update?`, `destroy?` remain restricted to the owning org's admins. @@ -228,9 +318,7 @@ The workspace project policy must allow guest org admins to view (but not edit) - `manage_share?` — org_one admin only (view shared orgs, disconnect guest org) - `disconnect_share?` — org_two admin (disconnect own org) -### Task & Client Scope Adjustments - -**TaskPolicy scope:** Extended to include org_one's tasks when the user is on a shared project. When building task lists for time_reg forms on shared projects, include tasks from the project's owning org (via `assigned_tasks` on the project). +### Client Scope **ClientPolicy scope:** Not extended — guest orgs do not browse org_one's client list. Shared projects appear directly in the project list. Report filters for guest orgs show the shared project directly without the client hierarchy. @@ -273,6 +361,8 @@ end Reports and billing call `used_rate_for(current_organization)` instead of `used_rate`. The existing `used_rate` continues to work for contexts where only the owning org's rate matters. +**Reports refactoring:** `Reports::Summary#total_billable_amount` and `Reports::Result` both call `billed_amount` which uses `used_rate` without org context. These must be updated to accept an organization parameter and use `used_rate_for(organization)`. The `billed_amount` method should gain a `billed_amount_for(organization)` variant following the same pattern. + ### AssignedTask Rate-Change Archiving When `AssignedTask#handle_rate_change` archives an old record and creates a new one, `ProjectShareTaskRate` records referencing the old `assigned_task_id` must be migrated: @@ -302,11 +392,11 @@ This is separate from the project edit page (which org_two cannot access). **`Workspace::ProjectSharesController`** — manages the org-to-org relationship: - `index` — list guest orgs for a project (org_one admin) or list shared projects (org_two admin) -- `update` — org_two admin updates their rates on the `ProjectShare` +- `update_rates` — org_two admin bulk-updates their project rate + task rates on the `ProjectShare` - `destroy` — either side disconnects -**`Workspace::ProjectShareTaskRatesController`** — manages per-task rates for guest orgs: -- `create` / `update` — org_two admin sets task rates +**`Workspace::ProjectShareTaskRatesController`** — manages individual per-task rates for guest orgs: +- `create` / `update` — org_two admin sets individual task rates (used by the `update_rates` form) - Nested under project_share routes ### Existing Controller Changes From 3e2bfc99c295030682be22d0dabf4f515813cfb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 13:50:24 +0100 Subject: [PATCH 05/31] Fix lint --- app/views/components/combobox_component/content.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/components/combobox_component/content.rb b/app/views/components/combobox_component/content.rb index 2c78b865..9cd71cbd 100644 --- a/app/views/components/combobox_component/content.rb +++ b/app/views/components/combobox_component/content.rb @@ -8,7 +8,7 @@ def initialize(**attrs) super end - def view_template(&) + def view_template(&block) div(**@attrs) do div( data: { @@ -16,7 +16,7 @@ def view_template(&) wrapper_id: @wrapper_id, action: "keydown.up->combobox-content#handleKeyUp keydown.down->combobox-content#handleKeyDown keydown.enter->combobox-content#handleKeyEnter keydown.esc->combobox-content#handleKeyEsc" }, - class: "flex h-full w-full flex-col overflow-hidden rounded-md rounded-lg border shadow-md bg-white", & + class: "flex h-full w-full flex-col overflow-hidden rounded-md rounded-lg border shadow-md bg-white", &block ) end end From b6728d7c9ecf742ad436a08a22d4996fd6485120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 14:27:44 +0100 Subject: [PATCH 06/31] Setup local CI --- .gitignore | 2 + Gemfile | 5 + Gemfile.lock | 25 +++ bin/ci | 5 + config/brakeman.ignore | 18 ++ config/ci.rb | 23 +++ lib/ci_runner.rb | 161 ++++++++++++++++++ test/test_helper.rb | 17 ++ .../combobox_component/content_test.rb | 15 ++ 9 files changed, 271 insertions(+) create mode 100755 bin/ci create mode 100644 config/brakeman.ignore create mode 100644 config/ci.rb create mode 100644 lib/ci_runner.rb create mode 100644 test/views/components/combobox_component/content_test.rb diff --git a/.gitignore b/.gitignore index 068d86a0..1cf52baf 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,5 @@ .DS_Store /dump.rdb + +/coverage diff --git a/Gemfile b/Gemfile index 1feb8ae8..32b05f10 100644 --- a/Gemfile +++ b/Gemfile @@ -91,6 +91,9 @@ group :test do gem "selenium-webdriver" gem "webdrivers" gem "rails-controller-testing" + gem "simplecov" + gem "simplecov-lcov" + gem "undercover" end gem "sentry-ruby" @@ -123,3 +126,5 @@ gem "tailwind_merge", "~> 1.2" gem "wicked", "~> 2.0" gem "fast-mcp" + +gem "brakeman", "~> 8.0" diff --git a/Gemfile.lock b/Gemfile.lock index d85d9306..03e435f1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,6 +92,8 @@ GEM bindex (0.8.1) bootsnap (1.18.6) msgpack (~> 1.2) + brakeman (8.0.4) + racc builder (3.2.4) capybara (3.39.0) addressable @@ -124,6 +126,7 @@ GEM devise (>= 4.6) discard (1.3.0) activerecord (>= 4.2, < 8) + docile (1.4.1) dotenv (2.8.1) dotenv-rails (2.8.1) dotenv (= 2.8.1) @@ -180,6 +183,8 @@ GEM railties (>= 7.0.0) i18n (1.14.4) concurrent-ruby (~> 1.0) + imagen (0.2.0) + parser (>= 2.5, != 2.5.1.1) invisible_captcha (2.3.0) rails (>= 5.2) io-console (0.6.0) @@ -354,6 +359,7 @@ GEM rubyzip (2.3.2) rufus-scheduler (3.9.1) fugit (~> 1.1, >= 1.1.6) + rugged (1.9.0) securerandom (0.4.1) selenium-webdriver (4.9.0) rexml (~> 3.2, >= 3.2.5) @@ -376,6 +382,13 @@ GEM rufus-scheduler (~> 3.2) sidekiq (>= 6, < 8) tilt (>= 1.4.0, < 3) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.13.2) + simplecov-lcov (0.9.0) + simplecov_json_formatter (0.1.4) sin_lru_redux (2.5.2) sprockets (4.2.0) concurrent-ruby (~> 1.0) @@ -398,6 +411,14 @@ GEM railties (>= 7.1.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + undercover (0.8.4) + base64 + bigdecimal + imagen (>= 0.2.0) + rainbow (>= 2.1, < 4.0) + rugged (>= 0.27, < 1.10) + simplecov + simplecov_json_formatter unicode-display_width (2.5.0) warden (1.2.9) rack (>= 2.0.9) @@ -431,6 +452,7 @@ DEPENDENCIES action_policy activerecord-import bootsnap + brakeman (~> 8.0) capybara cssbundling-rails debug @@ -461,12 +483,15 @@ DEPENDENCIES sentry-ruby sidekiq (~> 7.3) sidekiq-scheduler + simplecov + simplecov-lcov sprockets-rails stimulus-rails strong_migrations tailwind_merge (~> 1.2) turbo-rails (>= 2.0.10) tzinfo-data + undercover web-console webdrivers wicked (~> 2.0) diff --git a/bin/ci b/bin/ci new file mode 100755 index 00000000..5db3a627 --- /dev/null +++ b/bin/ci @@ -0,0 +1,5 @@ +#!/usr/bin/env ruby +require_relative '../lib/ci_runner' + +CI = CIRunner +require_relative '../config/ci' diff --git a/config/brakeman.ignore b/config/brakeman.ignore new file mode 100644 index 00000000..a5d4fbc6 --- /dev/null +++ b/config/brakeman.ignore @@ -0,0 +1,18 @@ +{ + "ignored_warnings": [ + { + "fingerprint": "6f5239fb87c64764d0c209014deb5cf504c2c10ee424bd33590f0a4f22e01d8f", + "note": "CSRF protection is handled by Devise and Rails defaults" + }, + { + "fingerprint": "561be2095bfe3ca8d4e195b0ce29babadf272599e070f44908d877f8c695a4ff", + "note": "Role assignment is authorized via ActionPolicy" + }, + { + "fingerprint": "dae417d130b549c5dcc6064473056341c812c8a2dc2b126af8ae06b095aa532b", + "note": "Role assignment is authorized via ActionPolicy" + } + ], + "updated": "2026-03-17", + "brakeman_version": "8.0.4" +} diff --git a/config/ci.rb b/config/ci.rb new file mode 100644 index 00000000..1bc02ff1 --- /dev/null +++ b/config/ci.rb @@ -0,0 +1,23 @@ +# Run using bin/ci +# Options: bin/ci --fail-fast, bin/ci --signoff + +signoff = ARGV.include?("--signoff") || ARGV.include?("-s") +fail_fast = ARGV.include?("--fail-fast") || ARGV.include?("-f") + +ci = CI.run("Continuous Integration", "Running checks...", fail_fast: fail_fast) do + step "Rubocop", "bundle", "exec", "rubocop", "-A" + # step "Prettier", "yarn", "prettier", "--config", ".prettierrc.json", "app/packs", "app/components", "--write" + step "Brakeman", "bundle", "exec", "brakeman", "--quiet", "--no-pager", "--except=EOLRails" + step "Minitest", "bundle", "exec", "rails", "test" + step "Undercover", "bundle", "exec", "undercover", "--lcov", "coverage/lcov/app.lcov", "--compare", "origin/main" +end + +if ci.success? + if signoff + puts "\nRunning gh signoff..." + system("gh", "signoff") || warn("gh signoff failed - is gh-signoff installed?") + end + exit 0 +else + exit 1 +end diff --git a/lib/ci_runner.rb b/lib/ci_runner.rb new file mode 100644 index 00000000..cee81080 --- /dev/null +++ b/lib/ci_runner.rb @@ -0,0 +1,161 @@ +# Adapted from Rails 8.1 ActiveSupport::ContinuousIntegration +# https://github.com/rails/rails/blob/8-1-stable/activesupport/lib/active_support/continuous_integration.rb +require "open3" + +class CIRunner + COLORS = { + banner: "\e[1;32m", # Bold green + title: "\e[1;35m", # Bold purple + subtitle: "\e[1;90m", # Bold gray + error: "\e[1;31m", # Bold red + success: "\e[1;32m", # Bold green + reset: "\e[0m" + }.freeze + + attr_reader :results + + def initialize(output: $stdout, fail_fast: false) + @output = output + @fail_fast = fail_fast + @results = [] + end + + def self.run(title = "Continuous Integration", subtitle = "Running checks...", output: $stdout, fail_fast: false, &block) + new(output: output, fail_fast: fail_fast).tap do |ci| + ci.heading(title, subtitle, type: :banner, padding: false) + ci.report(title, &block) + end + end + + def step(title, *command) + heading(title, command.join(" "), type: :title) + + started_at = Time.now + stdout, stderr, status = run_with_timer(title, started_at) do + Open3.capture3(*command) + end + elapsed = format_elapsed(Time.now - started_at) + success = status.success? + + if success + echo("\u2705 #{title} passed in #{elapsed}", type: :success) + else + echo("\u274C #{title} failed in #{elapsed}", type: :error) + print_failure_output(stdout, stderr) + end + + results << [ success, title, stdout, stderr ] + end + + def success? + results.all?(&:first) + end + + def failure(title, subtitle = nil) + heading(title, subtitle, type: :error) + end + + def heading(text, subtitle = nil, type: :banner, padding: true) + @output.puts if padding + echo(text, type: type) + echo(subtitle, type: :subtitle) if subtitle + @output.puts if padding + end + + def report(title, &block) + Signal.trap("INT") { abort colorize("\n\u274C #{title} interrupted", :error) } + + ci = self.class.new(output: @output, fail_fast: @fail_fast) + elapsed = timing { ci.instance_eval(&block) } + + if ci.success? + echo("\u2705 #{title} passed in #{elapsed}", type: :success) + else + echo("\u274C #{title} failed in #{elapsed}", type: :error) + abort if @fail_fast + + # List failed steps (output was already printed by each step) + ci.results.reject(&:first).each do |_, step_title, _, _| + echo(" \u21B3 #{step_title} failed", type: :error) + end + end + + results.concat(ci.results) + ensure + Signal.trap("INT", "DEFAULT") + end + + private + + def run_with_timer(title, started_at) + result = nil + stop_timer = false + + # Only show timer if output is a TTY + if @output.respond_to?(:tty?) && @output.tty? + timer_thread = Thread.new do + until stop_timer + elapsed = format_elapsed(Time.now - started_at) + @output.print colorize("\r⏱️ #{title} running... #{elapsed}", :subtitle) + sleep 0.1 + end + end + + result = yield + + stop_timer = true + timer_thread.join + @output.print "\r#{' ' * 60}\r" # Clear the timer line + else + result = yield + end + + result + end + + def print_failure_output(stdout, stderr) + output_text = extract_relevant_output(stdout, stderr) + @output.puts output_text unless output_text.empty? + end + + def extract_relevant_output(stdout, stderr) + combined = "#{stdout}#{stderr}" + + # For RSpec output, extract from "Failures:" onward + if combined.include?("Failures:") + combined.slice(/Failures:.*Failed examples:.*?(?=\n\n|\z)/m) || + combined.slice(/Failures:.*/m) || + combined + else + combined + end + end + + def echo(text, type:) + @output.puts colorize(text, type) + end + + def colorize(text, type) + if @output.respond_to?(:tty?) && @output.tty? + "#{COLORS[type]}#{text}#{COLORS[:reset]}" + else + text + end + end + + def timing + started_at = Time.now + yield + format_elapsed(Time.now - started_at) + end + + def format_elapsed(elapsed) + if elapsed >= 60 + minutes = (elapsed / 60).to_i + seconds = elapsed % 60 + "#{minutes}m#{format('%.2fs', seconds)}" + else + format("%.2fs", elapsed) + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 12bd5dde..01732f20 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,3 +1,20 @@ +require "simplecov" +require "simplecov-lcov" + +SimpleCov::Formatter::LcovFormatter.config do |c| + c.report_with_single_file = true + c.single_report_path = "coverage/lcov/app.lcov" +end + +SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter.new([ + SimpleCov::Formatter::HTMLFormatter, + SimpleCov::Formatter::LcovFormatter +]) + +SimpleCov.start "rails" do + enable_coverage :branch +end + ENV["RAILS_ENV"] ||= "test" require_relative "../config/environment" require "rails/test_help" diff --git a/test/views/components/combobox_component/content_test.rb b/test/views/components/combobox_component/content_test.rb new file mode 100644 index 00000000..05f71c68 --- /dev/null +++ b/test/views/components/combobox_component/content_test.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require "test_helper" + +module ComboboxComponent + class ContentTest < ActiveSupport::TestCase + test "renders content wrapper with combobox data attributes" do + output = Content.new(wrapper_id: "test-wrapper").call { "inner content" } + + assert_includes output, "inner content" + assert_includes output, 'data-controller="combobox-content"' + assert_includes output, 'data-wrapper-id="test-wrapper"' + end + end +end From 6d31d166e438b3bf453975c9f27830d01d900adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 14:37:38 +0100 Subject: [PATCH 07/31] feat: add ProjectShare model and migration Create the foundational model for org-to-org project sharing. ProjectShare links a project to a guest organization with an optional rate override, enforcing uniqueness and preventing sharing with the owning organization. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/models/organization.rb | 2 + app/models/project.rb | 2 + app/models/project_share.rb | 17 +++++++ .../20260317133328_create_project_shares.rb | 13 ++++++ db/schema.rb | 15 ++++++- test/fixtures/project_shares.yml | 4 ++ test/models/project_share_test.rb | 44 +++++++++++++++++++ 7 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 app/models/project_share.rb create mode 100644 db/migrate/20260317133328_create_project_shares.rb create mode 100644 test/fixtures/project_shares.yml create mode 100644 test/models/project_share_test.rb diff --git a/app/models/organization.rb b/app/models/organization.rb index 3fdc6278..d9f7f864 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -6,6 +6,8 @@ class Organization < ApplicationRecord has_many :assigned_tasks, through: :tasks has_many :projects, through: :clients has_many :time_regs, through: :users + has_many :project_shares, dependent: :destroy + has_many :shared_projects, through: :project_shares, source: :project validates :name, presence: true, uniqueness: true validate :currency_exists diff --git a/app/models/project.rb b/app/models/project.rb index d865b645..b2e5cf60 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -18,6 +18,8 @@ class Project < ApplicationRecord has_many :access_infos, through: :project_accesses has_many :users, through: :access_infos has_many :project_invitations, dependent: :destroy + has_many :project_shares, dependent: :destroy + has_many :shared_organizations, through: :project_shares, source: :organization accepts_nested_attributes_for :assigned_tasks, allow_destroy: true diff --git a/app/models/project_share.rb b/app/models/project_share.rb new file mode 100644 index 00000000..367d7ffc --- /dev/null +++ b/app/models/project_share.rb @@ -0,0 +1,17 @@ +class ProjectShare < ApplicationRecord + include RateConvertible + + belongs_to :project + belongs_to :organization + + has_many :project_share_task_rates, dependent: :destroy + + validates :organization_id, uniqueness: { scope: :project_id } + validate :organization_is_not_project_owner + + private + + def organization_is_not_project_owner + errors.add(:organization, :is_project_owner) if organization_id == project&.organization&.id + end +end diff --git a/db/migrate/20260317133328_create_project_shares.rb b/db/migrate/20260317133328_create_project_shares.rb new file mode 100644 index 00000000..0ca7a9c5 --- /dev/null +++ b/db/migrate/20260317133328_create_project_shares.rb @@ -0,0 +1,13 @@ +class CreateProjectShares < ActiveRecord::Migration[7.1] + def change + create_table :project_shares do |t| + t.references :project, null: false, foreign_key: true + t.references :organization, null: false, foreign_key: true + t.integer :rate, default: 0, null: false + + t.timestamps + end + + add_index :project_shares, [ :project_id, :organization_id ], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a44cc63..78018a29 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2026_02_27_140001) do +ActiveRecord::Schema[7.1].define(version: 2026_03_17_133328) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -85,6 +85,17 @@ t.index ["project_id"], name: "index_project_invitations_on_project_id" end + create_table "project_shares", force: :cascade do |t| + t.bigint "project_id", null: false + t.bigint "organization_id", null: false + t.integer "rate", default: 0, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["organization_id"], name: "index_project_shares_on_organization_id" + t.index ["project_id", "organization_id"], name: "index_project_shares_on_project_id_and_organization_id", unique: true + t.index ["project_id"], name: "index_project_shares_on_project_id" + end + create_table "projects", force: :cascade do |t| t.string "name" t.text "description" @@ -172,6 +183,8 @@ add_foreign_key "project_invitations", "access_infos", column: "accepted_as_access_info_id" add_foreign_key "project_invitations", "projects" add_foreign_key "project_invitations", "users", column: "invited_by_id" + add_foreign_key "project_shares", "organizations" + add_foreign_key "project_shares", "projects" add_foreign_key "projects", "clients" add_foreign_key "tasks", "organizations" add_foreign_key "time_regs", "assigned_tasks" diff --git a/test/fixtures/project_shares.yml b/test/fixtures/project_shares.yml new file mode 100644 index 00000000..0f855395 --- /dev/null +++ b/test/fixtures/project_shares.yml @@ -0,0 +1,4 @@ +project_one_shared_with_org_two: + project: project_1 + organization: organization_two + rate: 0 diff --git a/test/models/project_share_test.rb b/test/models/project_share_test.rb new file mode 100644 index 00000000..204878b0 --- /dev/null +++ b/test/models/project_share_test.rb @@ -0,0 +1,44 @@ +require "test_helper" + +class ProjectShareTest < ActiveSupport::TestCase + def setup + @organization_one = organizations(:organization_one) + @organization_two = organizations(:organization_two) + @organization_three = organizations(:organization_three) + @project = projects(:project_1) + end + + test "valid project share" do + project_share = ProjectShare.new(project: @project, organization: @organization_three, rate: 250) + assert project_share.valid? + end + + test "validates uniqueness of organization scoped to project" do + # Fixture already creates project_1 shared with organization_two + duplicate = ProjectShare.new(project: @project, organization: @organization_two, rate: 0) + assert_not duplicate.valid? + assert_includes duplicate.errors[:organization_id], "has already been taken" + end + + test "validates that organization cannot be the project owning organization" do + project_share = ProjectShare.new(project: @project, organization: @organization_one) + assert_not project_share.valid? + assert project_share.errors.added?(:organization, :is_project_owner) + end + + test "rate_currency getter converts hundredths to currency format" do + project_share = ProjectShare.new(rate: 250) + assert_equal "2,50", project_share.rate_currency + end + + test "rate_currency setter converts currency format to hundredths" do + project_share = ProjectShare.new + project_share.rate_currency = "2,50" + assert_equal 250, project_share.rate + end + + test "rate defaults to 0" do + project_share = ProjectShare.new(project: @project, organization: @organization_three) + assert_equal 0, project_share.rate + end +end From 7d7a4e00f942c64bf806aa37892fb7869a6e5f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 14:47:13 +0100 Subject: [PATCH 08/31] feat: add ProjectShareTaskRate model and migration Add per-task rate override model for shared projects. Includes RateConvertible concern, uniqueness validation scoped to project_share, and a unique composite index on (project_share_id, assigned_task_id). Co-Authored-By: Claude Opus 4.6 (1M context) --- app/models/project_share_task_rate.rb | 8 +++ ...7134303_create_project_share_task_rates.rb | 14 +++++ db/schema.rb | 15 ++++- test/fixtures/project_share_task_rates.yml | 4 ++ test/models/project_share_task_rate_test.rb | 61 +++++++++++++++++++ 5 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 app/models/project_share_task_rate.rb create mode 100644 db/migrate/20260317134303_create_project_share_task_rates.rb create mode 100644 test/fixtures/project_share_task_rates.yml create mode 100644 test/models/project_share_task_rate_test.rb diff --git a/app/models/project_share_task_rate.rb b/app/models/project_share_task_rate.rb new file mode 100644 index 00000000..11efaa15 --- /dev/null +++ b/app/models/project_share_task_rate.rb @@ -0,0 +1,8 @@ +class ProjectShareTaskRate < ApplicationRecord + include RateConvertible + + belongs_to :project_share + belongs_to :assigned_task + + validates :assigned_task_id, uniqueness: { scope: :project_share_id } +end diff --git a/db/migrate/20260317134303_create_project_share_task_rates.rb b/db/migrate/20260317134303_create_project_share_task_rates.rb new file mode 100644 index 00000000..0befd341 --- /dev/null +++ b/db/migrate/20260317134303_create_project_share_task_rates.rb @@ -0,0 +1,14 @@ +class CreateProjectShareTaskRates < ActiveRecord::Migration[7.1] + def change + create_table :project_share_task_rates do |t| + t.references :project_share, null: false, foreign_key: true + t.references :assigned_task, null: false, foreign_key: true + t.integer :rate, default: 0, null: false + + t.timestamps + end + + add_index :project_share_task_rates, [ :project_share_id, :assigned_task_id ], + unique: true, name: "idx_project_share_task_rates_unique" + end +end diff --git a/db/schema.rb b/db/schema.rb index 78018a29..abcbea72 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2026_03_17_133328) do +ActiveRecord::Schema[7.1].define(version: 2026_03_17_134303) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -85,6 +85,17 @@ t.index ["project_id"], name: "index_project_invitations_on_project_id" end + create_table "project_share_task_rates", force: :cascade do |t| + t.bigint "project_share_id", null: false + t.bigint "assigned_task_id", null: false + t.integer "rate", default: 0, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["assigned_task_id"], name: "index_project_share_task_rates_on_assigned_task_id" + t.index ["project_share_id", "assigned_task_id"], name: "idx_project_share_task_rates_unique", unique: true + t.index ["project_share_id"], name: "index_project_share_task_rates_on_project_share_id" + end + create_table "project_shares", force: :cascade do |t| t.bigint "project_id", null: false t.bigint "organization_id", null: false @@ -183,6 +194,8 @@ add_foreign_key "project_invitations", "access_infos", column: "accepted_as_access_info_id" add_foreign_key "project_invitations", "projects" add_foreign_key "project_invitations", "users", column: "invited_by_id" + add_foreign_key "project_share_task_rates", "assigned_tasks" + add_foreign_key "project_share_task_rates", "project_shares" add_foreign_key "project_shares", "organizations" add_foreign_key "project_shares", "projects" add_foreign_key "projects", "clients" diff --git a/test/fixtures/project_share_task_rates.yml b/test/fixtures/project_share_task_rates.yml new file mode 100644 index 00000000..d17d21bc --- /dev/null +++ b/test/fixtures/project_share_task_rates.yml @@ -0,0 +1,4 @@ +task_rate_one: + project_share: project_one_shared_with_org_two + assigned_task: task_1 + rate: 300 diff --git a/test/models/project_share_task_rate_test.rb b/test/models/project_share_task_rate_test.rb new file mode 100644 index 00000000..8dee7ced --- /dev/null +++ b/test/models/project_share_task_rate_test.rb @@ -0,0 +1,61 @@ +require "test_helper" + +class ProjectShareTaskRateTest < ActiveSupport::TestCase + def setup + @project_share = project_shares(:project_one_shared_with_org_two) + @assigned_task = assigned_task(:task_1) + end + + test "valid project share task rate" do + rate = ProjectShareTaskRate.new( + project_share: @project_share, + assigned_task: assigned_task(:task_2), + rate: 300 + ) + assert rate.valid? + end + + test "belongs to project_share" do + rate = project_share_task_rates(:task_rate_one) + assert_instance_of ProjectShare, rate.project_share + end + + test "belongs to assigned_task" do + rate = project_share_task_rates(:task_rate_one) + assert_instance_of AssignedTask, rate.assigned_task + end + + test "validates uniqueness of assigned_task_id scoped to project_share_id" do + existing = project_share_task_rates(:task_rate_one) + duplicate = ProjectShareTaskRate.new( + project_share: existing.project_share, + assigned_task: existing.assigned_task, + rate: 500 + ) + assert_not duplicate.valid? + assert_includes duplicate.errors[:assigned_task_id], "has already been taken" + end + + test "includes RateConvertible" do + assert_includes ProjectShareTaskRate.ancestors, RateConvertible + end + + test "rate_currency getter converts hundredths to currency format" do + rate = ProjectShareTaskRate.new(rate: 350) + assert_equal "3,50", rate.rate_currency + end + + test "rate_currency setter converts currency format to hundredths" do + rate = ProjectShareTaskRate.new + rate.rate_currency = "4,25" + assert_equal 425, rate.rate + end + + test "rate defaults to 0" do + rate = ProjectShareTaskRate.new( + project_share: @project_share, + assigned_task: @assigned_task + ) + assert_equal 0, rate.rate + end +end From d2c7d254cff4a3508aed5938ae971cdabf874c08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 14:51:31 +0100 Subject: [PATCH 09/31] feat: auto-create ProjectShare on invitation acceptance When accepting a project invitation into an organization different from the project's owner, automatically create a ProjectShare linking the project to the guest organization via find_or_create_by! to handle duplicates gracefully. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/models/project_invitation.rb | 3 ++ .../project_invitation_service_test.rb | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/app/models/project_invitation.rb b/app/models/project_invitation.rb index c8632bbc..405d6c60 100644 --- a/app/models/project_invitation.rb +++ b/app/models/project_invitation.rb @@ -68,6 +68,9 @@ def accept!(organization) access_info: access_info ) + # Create project share for cross-org access + ProjectShare.find_or_create_by!(project: project, organization: organization) if organization != project.organization + # Mark invitation as accepted update!( accepted_at: Time.current, diff --git a/test/services/project_invitation_service_test.rb b/test/services/project_invitation_service_test.rb index 1cd4de49..34ba4261 100644 --- a/test/services/project_invitation_service_test.rb +++ b/test/services/project_invitation_service_test.rb @@ -47,4 +47,40 @@ def setup assert project_access, "ProjectAccess should be created" assert_equal @external_email, project_access.user.email end + + test "accepting invitation creates ProjectShare when user org differs from project org" do + # Use a different project to avoid fixture collision + project = projects(:project_2) + invitation = ProjectInvitation.create!( + project: project, + invited_email: @external_email, + invited_by: @admin_user, + invited_at: Time.current + ) + + assert_difference "ProjectShare.count", 1 do + ProjectInvitationService.accept_invitation(invitation.invitation_token, @external_org) + end + + project_share = ProjectShare.find_by(project: project, organization: @external_org) + assert project_share, "ProjectShare should be created" + assert_equal 0, project_share.rate + end + + test "accepting invitation reuses existing ProjectShare if one already exists" do + # project_1 already has a ProjectShare with organization_two via fixtures + invitation = ProjectInvitation.create!( + project: @project, + invited_email: @external_email, + invited_by: @admin_user, + invited_at: Time.current + ) + + assert_no_difference "ProjectShare.count" do + ProjectInvitationService.accept_invitation(invitation.invitation_token, @external_org) + end + + project_share = ProjectShare.find_by(project: @project, organization: @external_org) + assert project_share, "Existing ProjectShare should still exist" + end end From 0f6682a529d4f911769e9f4df05f6209840d3bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 14:57:16 +0100 Subject: [PATCH 10/31] feat: migrate ProjectShareTaskRate on AssignedTask rate change When an AssignedTask's rate changes, the existing record is archived and a new one is created. This ensures any associated ProjectShareTaskRate records are migrated to point to the new AssignedTask. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/models/assigned_task.rb | 3 ++- test/models/assigned_task_test.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/models/assigned_task.rb b/app/models/assigned_task.rb index d6c72cf4..9c86e02d 100644 --- a/app/models/assigned_task.rb +++ b/app/models/assigned_task.rb @@ -41,7 +41,8 @@ def is_not_archived def handle_rate_change if rate_changed? - self.class.create!(project: project, task: task, rate: rate) + new_task = self.class.create!(project: project, task: task, rate: rate) + ProjectShareTaskRate.where(assigned_task: self).update_all(assigned_task_id: new_task.id) self.is_archived = true self.rate = rate_was end diff --git a/test/models/assigned_task_test.rb b/test/models/assigned_task_test.rb index 915e3672..4237fbcf 100644 --- a/test/models/assigned_task_test.rb +++ b/test/models/assigned_task_test.rb @@ -45,4 +45,23 @@ class AssignedTaskTest < ActiveSupport::TestCase assert_equal last_assigned_task.project, @project assert_equal last_assigned_task.task, @task end + + test "should migrate ProjectShareTaskRate records to new assigned task when rate changes" do + assigned_task = assigned_task(:task_1) + project_share_task_rate = project_share_task_rates(:task_rate_one) + + assert_equal assigned_task, project_share_task_rate.assigned_task + + # update rate to trigger archiving + assert_difference("AssignedTask.count") do + assigned_task.update!(rate: 500) + end + + new_assigned_task = AssignedTask.where(project: assigned_task.project, task: assigned_task.task).active_task.first + + # ProjectShareTaskRate should now point to the new assigned task + project_share_task_rate.reload + assert_equal new_assigned_task, project_share_task_rate.assigned_task + assert_not_equal assigned_task, project_share_task_rate.assigned_task + end end From b9f94e7a67a4c98e1c80ba2c477280164bd44479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:02:50 +0100 Subject: [PATCH 11/31] feat: add ProjectSharePolicy Authorize show/update/destroy actions on project shares with proper multi-tenant scoping. Owner org admins can view and disconnect shares; guest org admins can view, manage rates, and disconnect. Non-admin users are denied all access. Scope returns shares visible to the current organization (as guest or owner). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../workspace/project_share_policy.rb | 47 +++++++ .../workspace/project_share_policy_test.rb | 129 ++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 app/policies/workspace/project_share_policy.rb create mode 100644 test/policies/workspace/project_share_policy_test.rb diff --git a/app/policies/workspace/project_share_policy.rb b/app/policies/workspace/project_share_policy.rb new file mode 100644 index 00000000..4a84be3b --- /dev/null +++ b/app/policies/workspace/project_share_policy.rb @@ -0,0 +1,47 @@ +module Workspace + class ProjectSharePolicy < WorkspacePolicy + def show? + user.organization_admin? && owner_or_guest_admin? + end + + def update? + user.organization_admin? && guest_org_admin? + end + + def destroy? + user.organization_admin? && owner_or_guest_admin? + end + + scope_for :relation do |relation| + if user.organization_admin? + relation + .where(organization: user.current_organization) + .or( + relation.where( + project: Project.joins(client: :organization) + .where(organizations: { id: user.current_organization.id }) + ) + ) + else + relation.none + end + end + + private + + # User is admin of the organization that owns the project + def owner_org_admin? + record.project.organization == user.current_organization + end + + # User is admin of the guest organization + def guest_org_admin? + record.organization == user.current_organization + end + + # User is admin of either the owning or guest organization + def owner_or_guest_admin? + owner_org_admin? || guest_org_admin? + end + end +end diff --git a/test/policies/workspace/project_share_policy_test.rb b/test/policies/workspace/project_share_policy_test.rb new file mode 100644 index 00000000..20b99a93 --- /dev/null +++ b/test/policies/workspace/project_share_policy_test.rb @@ -0,0 +1,129 @@ +require "test_helper" + +class Workspace::ProjectSharePolicyTest < ActiveSupport::TestCase + fixtures :all + + setup do + @project_share = project_shares(:project_one_shared_with_org_two) + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + end + + # Helper to build a policy instance for a given user and record + def policy_for(user, record = @project_share) + Workspace::ProjectSharePolicy.new(record, user: user) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- show? --- + + test "show? allowed for org_one admin (project owner)" do + admin = users(:organization_admin) + # organization_admin is active in org_one by default + assert_equal @org_one, admin.current_organization + assert policy_for(admin).apply(:show?) + end + + test "show? allowed for org_two admin (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + assert policy_for(admin).apply(:show?) + end + + test "show? denied for non-admin user in org_one" do + joe = users(:joe) + assert_equal @org_one, joe.current_organization + assert_not policy_for(joe).apply(:show?) + end + + test "show? denied for non-admin user in org_two" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + assert_not policy_for(ron).apply(:show?) + end + + # --- update? --- + + test "update? allowed for org_two admin (guest org managing rates)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + assert policy_for(admin).apply(:update?) + end + + test "update? denied for org_one admin (project owner cannot manage guest rates)" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + assert_not policy_for(admin).apply(:update?) + end + + test "update? denied for non-admin user in org_two" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + assert_not policy_for(ron).apply(:update?) + end + + # --- destroy? --- + + test "destroy? allowed for org_one admin (project owner)" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + assert policy_for(admin).apply(:destroy?) + end + + test "destroy? allowed for org_two admin (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + assert policy_for(admin).apply(:destroy?) + end + + test "destroy? denied for non-admin user in org_one" do + joe = users(:joe) + assert_equal @org_one, joe.current_organization + assert_not policy_for(joe).apply(:destroy?) + end + + test "destroy? denied for non-admin user in org_two" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + assert_not policy_for(ron).apply(:destroy?) + end + + # --- scope --- + + test "scope returns project_shares for admin of org_two (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + + policy = Workspace::ProjectSharePolicy.new(user: admin) + scope = policy.apply_scope(ProjectShare.all, type: :relation) + + assert_includes scope, @project_share + end + + test "scope returns project_shares for admin of org_one (project owner)" do + admin = users(:organization_admin) + # Active in org_one by default + + policy = Workspace::ProjectSharePolicy.new(user: admin) + scope = policy.apply_scope(ProjectShare.all, type: :relation) + + assert_includes scope, @project_share + end + + test "scope returns empty for non-admin user" do + joe = users(:joe) + + policy = Workspace::ProjectSharePolicy.new(user: joe) + scope = policy.apply_scope(ProjectShare.all, type: :relation) + + assert_empty scope + end +end From a9f7abc01cd147d823e41ae061d8733cf9ee5b4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:06:38 +0100 Subject: [PATCH 12/31] feat: add ProjectShareTaskRatePolicy Co-Authored-By: Claude Opus 4.6 (1M context) --- .../project_share_task_rate_policy.rb | 30 +++++ .../project_share_task_rate_policy_test.rb | 123 ++++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 app/policies/workspace/project_share_task_rate_policy.rb create mode 100644 test/policies/workspace/project_share_task_rate_policy_test.rb diff --git a/app/policies/workspace/project_share_task_rate_policy.rb b/app/policies/workspace/project_share_task_rate_policy.rb new file mode 100644 index 00000000..ebca9668 --- /dev/null +++ b/app/policies/workspace/project_share_task_rate_policy.rb @@ -0,0 +1,30 @@ +module Workspace + class ProjectShareTaskRatePolicy < WorkspacePolicy + def create? + user.organization_admin? && guest_org_admin? + end + + def update? + user.organization_admin? && guest_org_admin? + end + + def destroy? + user.organization_admin? && guest_org_admin? + end + + scope_for :relation do |relation| + if user.organization_admin? + relation.joins(:project_share).where(project_shares: { organization_id: user.current_organization.id }) + else + relation.none + end + end + + private + + # User is admin of the guest organization that owns this task rate + def guest_org_admin? + record.project_share.organization == user.current_organization + end + end +end diff --git a/test/policies/workspace/project_share_task_rate_policy_test.rb b/test/policies/workspace/project_share_task_rate_policy_test.rb new file mode 100644 index 00000000..ed1403b4 --- /dev/null +++ b/test/policies/workspace/project_share_task_rate_policy_test.rb @@ -0,0 +1,123 @@ +require "test_helper" + +class Workspace::ProjectShareTaskRatePolicyTest < ActiveSupport::TestCase + fixtures :all + + setup do + @task_rate = project_share_task_rates(:task_rate_one) + @project_share = project_shares(:project_one_shared_with_org_two) + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + end + + # Helper to build a policy instance for a given user and record + def policy_for(user, record = @task_rate) + Workspace::ProjectShareTaskRatePolicy.new(record, user: user) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- create? --- + + test "create? allowed for org_two admin (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + assert policy_for(admin).apply(:create?) + end + + test "create? denied for org_one admin (project owner)" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + assert_not policy_for(admin).apply(:create?) + end + + test "create? denied for non-admin user in org_two" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + assert_not policy_for(ron).apply(:create?) + end + + test "create? denied for non-admin user in org_one" do + joe = users(:joe) + assert_equal @org_one, joe.current_organization + assert_not policy_for(joe).apply(:create?) + end + + # --- update? --- + + test "update? allowed for org_two admin (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + assert policy_for(admin).apply(:update?) + end + + test "update? denied for org_one admin (project owner)" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + assert_not policy_for(admin).apply(:update?) + end + + test "update? denied for non-admin user in org_two" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + assert_not policy_for(ron).apply(:update?) + end + + # --- destroy? --- + + test "destroy? allowed for org_two admin (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + assert policy_for(admin).apply(:destroy?) + end + + test "destroy? denied for org_one admin (project owner)" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + assert_not policy_for(admin).apply(:destroy?) + end + + test "destroy? denied for non-admin user in org_two" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + assert_not policy_for(ron).apply(:destroy?) + end + + # --- scope --- + + test "scope returns task rates for admin of org_two (guest org)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + + policy = Workspace::ProjectShareTaskRatePolicy.new(user: admin) + scope = policy.apply_scope(ProjectShareTaskRate.all, type: :relation) + + assert_includes scope, @task_rate + end + + test "scope returns empty for admin of org_one (project owner has no task rates)" do + admin = users(:organization_admin) + # Active in org_one by default + + policy = Workspace::ProjectShareTaskRatePolicy.new(user: admin) + scope = policy.apply_scope(ProjectShareTaskRate.all, type: :relation) + + assert_not_includes scope, @task_rate + end + + test "scope returns empty for non-admin user" do + joe = users(:joe) + + policy = Workspace::ProjectShareTaskRatePolicy.new(user: joe) + scope = policy.apply_scope(ProjectShareTaskRate.all, type: :relation) + + assert_empty scope + end +end From 267fc3cc90533b78aea634631d5bfb9ac0a11b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:14:07 +0100 Subject: [PATCH 13/31] feat: extend ProjectPolicy scope for shared projects Update the Workspace::ProjectPolicy to support shared projects: - Scope now returns both own org projects and projects shared via ProjectShare, with non-admin users filtered by ProjectAccess - show? allows guest org admins to view shared projects (read-only) - edit?/update?/destroy? remain restricted to owning org admin - Add fixtures for cross-org test scenarios (org_two user, ProjectAccess) - Update spectator controller test for new redirect path Co-Authored-By: Claude Opus 4.6 (1M context) --- app/policies/workspace/project_policy.rb | 29 ++- .../workspace/projects_controller_test.rb | 2 +- test/fixtures/access_infos.yml | 6 + test/fixtures/project_accesses.yml | 4 + .../policies/workspace/project_policy_test.rb | 178 ++++++++++++++++++ .../project_invitation_service_test.rb | 3 +- 6 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 test/policies/workspace/project_policy_test.rb diff --git a/app/policies/workspace/project_policy.rb b/app/policies/workspace/project_policy.rb index 29ab7496..ffdf525e 100644 --- a/app/policies/workspace/project_policy.rb +++ b/app/policies/workspace/project_policy.rb @@ -4,18 +4,37 @@ class ProjectPolicy < WorkspacePolicy define_method("#{action}?") { user.organization_admin? } end - %i[ create show edit update destroy ].each do |action| + %i[ create edit update destroy ].each do |action| define_method("#{action}?") do user.organization_admin? && record.organization == user.current_organization end end + def show? + return true if user.organization_admin? && record.organization == user.current_organization + return true if user.organization_admin? && record.project_shares.exists?(organization: user.current_organization) + false + end + scope_for :relation do |relation| - if user.organization_admin? - relation.joins(:organization).where(organizations: { id: user.current_organization.id }).distinct - else - relation.none + organization = user.current_organization + + own_ids = relation.joins(client: :organization) + .where(organizations: { id: organization.id }) + .select(:id) + shared_ids = relation.joins(:project_shares) + .where(project_shares: { organization_id: organization.id }) + .select(:id) + combined = relation.where(id: own_ids).or(relation.where(id: shared_ids)) + + unless user.organization_admin? + combined = combined.where( + id: ProjectAccess.where(access_info: user.access_info(organization)) + .select(:project_id) + ) end + + combined.distinct end end end diff --git a/test/controllers/workspace/projects_controller_test.rb b/test/controllers/workspace/projects_controller_test.rb index 1a7f66f2..b54c67e0 100644 --- a/test/controllers/workspace/projects_controller_test.rb +++ b/test/controllers/workspace/projects_controller_test.rb @@ -62,7 +62,7 @@ class ProjectsControllerTest < ActionController::TestCase test "spectator should not show project" do sign_in users(:organization_spectator) get :show, params: { id: @project.id } - assert_redirected_to root_path + assert_redirected_to reports_path end test "should update project" do diff --git a/test/fixtures/access_infos.yml b/test/fixtures/access_infos.yml index 85e7ea5b..8196a2b5 100644 --- a/test/fixtures/access_infos.yml +++ b/test/fixtures/access_infos.yml @@ -46,6 +46,12 @@ access_info_org1_user: active: true role: 0 +access_info_org2_user: + user: organization_user + organization: organization_two + active: false + role: 0 + access_info_org1_spectator: user: organization_spectator organization: organization_one diff --git a/test/fixtures/project_accesses.yml b/test/fixtures/project_accesses.yml index be1bc9a9..6374abd2 100644 --- a/test/fixtures/project_accesses.yml +++ b/test/fixtures/project_accesses.yml @@ -6,4 +6,8 @@ ron_project_1: spectator_project_1: access_info: access_info_org1_spectator + project: project_1 + +ron_org2_shared_project_1: + access_info: access_info_2 project: project_1 \ No newline at end of file diff --git a/test/policies/workspace/project_policy_test.rb b/test/policies/workspace/project_policy_test.rb new file mode 100644 index 00000000..8a1dc962 --- /dev/null +++ b/test/policies/workspace/project_policy_test.rb @@ -0,0 +1,178 @@ +require "test_helper" + +class Workspace::ProjectPolicyTest < ActiveSupport::TestCase + fixtures :all + + setup do + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + @project_1 = projects(:project_1) # belongs to org_one, shared with org_two + @project_2 = projects(:project_2) # belongs to org_one, NOT shared + end + + # Helper to build a policy instance for a given user and record + def policy_for(user, record = Project) + Workspace::ProjectPolicy.new(record, user: user) + end + + # Helper to get the scope result for a given user + def scoped_projects(user) + policy = Workspace::ProjectPolicy.new(user: user) + policy.apply_scope(Project.all, type: :relation) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- Scope tests --- + + test "scope: org_one admin sees all org_one projects" do + admin = users(:organization_admin) + # active in org_one by default + assert_equal @org_one, admin.current_organization + + result = scoped_projects(admin) + + assert_includes result, @project_1 + assert_includes result, @project_2 + end + + test "scope: org_two admin sees projects shared with org_two" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + result = scoped_projects(admin) + + assert_includes result, @project_1 + end + + test "scope: org_two admin does NOT see org_one projects that are not shared" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + result = scoped_projects(admin) + + assert_not_includes result, @project_2 + end + + test "scope: org_two non-admin with ProjectAccess sees shared project" do + ron = users(:ron) + # ron is active in org_two by default (access_info_2) + assert_equal @org_two, ron.current_organization + + # ron has ProjectAccess to project_1 via access_info_2 (ron_org2_shared_project_1 fixture) + result = scoped_projects(ron) + + assert_includes result, @project_1 + end + + test "scope: org_two non-admin without ProjectAccess does NOT see shared project" do + org_user = users(:organization_user) + switch_org_context!(org_user, @org_two) + assert_equal @org_two, org_user.current_organization + + # organization_user has no ProjectAccess to project_1 in org_two + result = scoped_projects(org_user) + + assert_not_includes result, @project_1 + end + + test "scope: org_one non-admin with ProjectAccess sees own org project" do + ron = users(:ron) + switch_org_context!(ron, @org_one) + assert_equal @org_one, ron.current_organization + + # ron has ProjectAccess to project_1 via access_info_3 (ron_project_1 fixture) + result = scoped_projects(ron) + + assert_includes result, @project_1 + end + + test "scope: org_one non-admin without ProjectAccess does NOT see own org project" do + ron = users(:ron) + switch_org_context!(ron, @org_one) + assert_equal @org_one, ron.current_organization + + # ron does NOT have ProjectAccess to project_2 + result = scoped_projects(ron) + + assert_not_includes result, @project_2 + end + + # --- show? tests --- + + test "show? allows org_one admin to view own project" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + + assert policy_for(admin, @project_1).apply(:show?) + end + + test "show? allows org_two admin to view shared project (read-only)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert policy_for(admin, @project_1).apply(:show?) + end + + test "show? denies org_two admin for non-shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @project_2).apply(:show?) + end + + # --- edit?/update?/destroy? tests --- + + test "edit? denies org_two admin for shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @project_1).apply(:edit?) + end + + test "update? denies org_two admin for shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @project_1).apply(:update?) + end + + test "destroy? denies org_two admin for shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @project_1).apply(:destroy?) + end + + test "edit? allows org_one admin for own project" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + + assert policy_for(admin, @project_1).apply(:edit?) + end + + test "update? allows org_one admin for own project" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + + assert policy_for(admin, @project_1).apply(:update?) + end + + test "destroy? allows org_one admin for own project" do + admin = users(:organization_admin) + assert_equal @org_one, admin.current_organization + + assert policy_for(admin, @project_1).apply(:destroy?) + end +end diff --git a/test/services/project_invitation_service_test.rb b/test/services/project_invitation_service_test.rb index 34ba4261..daf37494 100644 --- a/test/services/project_invitation_service_test.rb +++ b/test/services/project_invitation_service_test.rb @@ -41,8 +41,9 @@ def setup assert_equal @external_org, invitation.accepted_as_access_info.organization # Test 4: Invited user has access to the project + invited_user = User.find_by(email: @external_email) project_access = ProjectAccess.joins(:access_info) - .where(project: @project, access_infos: { organization: @external_org }) + .where(project: @project, access_infos: { organization: @external_org, user: invited_user }) .first assert project_access, "ProjectAccess should be created" assert_equal @external_email, project_access.user.email From ddcce7caf920ae9d89c01c5ffb98f1d5968234c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:23:32 +0100 Subject: [PATCH 14/31] feat: extend TimeRegPolicy for shared projects Co-Authored-By: Claude Opus 4.6 (1M context) --- app/policies/time_reg_policy.rb | 28 ++++- test/policies/time_reg_policy_test.rb | 149 ++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 test/policies/time_reg_policy_test.rb diff --git a/app/policies/time_reg_policy.rb b/app/policies/time_reg_policy.rb index 37d339c4..e72c48d9 100644 --- a/app/policies/time_reg_policy.rb +++ b/app/policies/time_reg_policy.rb @@ -1,5 +1,10 @@ class TimeRegPolicy < ApplicationPolicy - %i[ show edit update destroy toggle_active export ].each do |action| + def show? + is_admin_allowed = user.organization_admin? && record.organization == user.current_organization + user == record.user || is_admin_allowed || admin_of_shared_project? + end + + %i[ edit update destroy toggle_active export ].each do |action| define_method("#{action}?") do is_admin_allowed = user.organization_admin? && record.organization == user.current_organization user == record.user || is_admin_allowed @@ -17,23 +22,28 @@ def create? same_organization = user.current_organization == record.organization no_organization = record.organization.nil? + shared_project = on_shared_project? if user.organization_admin? same_organization else - (same_organization || no_organization) && record.user == user + (same_organization || no_organization || shared_project) && record.user == user end end scope_for :relation do |relation| organization = user.current_organization + shared_project_ids = ProjectShare.where(organization_id: organization.id).select(:project_id) + if user.organization_admin? - relation.joins(:organization).where(organizations: { id: organization.id }).distinct + own_ids = relation.joins(:organization).where(organizations: { id: organization.id }).select(:id) + shared_ids = relation.joins(assigned_task: :project).where(projects: { id: shared_project_ids }).select(:id) + relation.where(id: own_ids).or(relation.where(id: shared_ids)).distinct elsif user.access_info.organization_spectator? projects = authorized_scope(Project.all, type: :relation).all relation.joins(:project).where(projects: projects).distinct else - relation.joins(:organization, :user).where(organizations: { id: organization.id }, users: { id: user.id }).distinct + relation.where(user: user).distinct end end @@ -41,4 +51,14 @@ def create? organization = user.current_organization relation.joins(:organization, :user).where(organizations: { id: organization.id }, users: { id: user.id }).distinct end + + private + + def on_shared_project? + ProjectShare.exists?(project: record.project, organization: user.current_organization) + end + + def admin_of_shared_project? + user.organization_admin? && on_shared_project? + end end diff --git a/test/policies/time_reg_policy_test.rb b/test/policies/time_reg_policy_test.rb new file mode 100644 index 00000000..982841b4 --- /dev/null +++ b/test/policies/time_reg_policy_test.rb @@ -0,0 +1,149 @@ +require "test_helper" + +class TimeRegPolicyTest < ActiveSupport::TestCase + fixtures :all + + setup do + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + @project_1 = projects(:project_1) # belongs to org_one, shared with org_two + @time_reg_joe = time_regs(:time_reg_1) # user=joe, project_1 (org_one's project) + @time_reg_ron = time_regs(:time_reg_2) # user=ron, project_1 (org_one's project) + end + + # Helper to build a policy instance for a given user and record + def policy_for(user, record) + TimeRegPolicy.new(record, user: user) + end + + # Helper to get the default scope result for a given user + def scoped_time_regs(user) + policy = TimeRegPolicy.new(user: user) + policy.apply_scope(TimeReg.all, type: :relation) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- create? --- + + test "create? org_two member can create their own time_reg on shared project" do + ron = users(:ron) + # ron is active in org_two by default + assert_equal @org_two, ron.current_organization + + # Build a new time_reg for ron on the shared project + new_time_reg = TimeReg.new( + user: ron, + assigned_task: assigned_task(:task_1), + date_worked: Date.today, + minutes: 60 + ) + + assert policy_for(ron, new_time_reg).apply(:create?) + end + + test "create? org_two admin CANNOT create time_regs on shared project (admin path checks same_organization)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + new_time_reg = TimeReg.new( + user: admin, + assigned_task: assigned_task(:task_1), + date_worked: Date.today, + minutes: 60 + ) + + # Admin path checks same_organization which fails because project belongs to org_one + assert_not policy_for(admin, new_time_reg).apply(:create?) + end + + # --- show? --- + + test "show? org_two admin can view any time_reg on shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + # Can see joe's time_reg on the shared project (not admin's own entry) + assert policy_for(admin, @time_reg_joe).apply(:show?) + end + + # --- edit?/update?/destroy? --- + + test "edit? org_two admin CANNOT edit org_one user's time_reg on shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @time_reg_joe).apply(:edit?) + end + + test "update? org_two admin CANNOT update org_one user's time_reg on shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @time_reg_joe).apply(:update?) + end + + test "destroy? org_two admin CANNOT destroy org_one user's time_reg on shared project" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + assert_not policy_for(admin, @time_reg_joe).apply(:destroy?) + end + + test "edit? org_two member CAN edit their own time_reg on shared project" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + + assert policy_for(ron, @time_reg_ron).apply(:edit?) + end + + test "update? org_two member CAN update their own time_reg on shared project" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + + assert policy_for(ron, @time_reg_ron).apply(:update?) + end + + test "destroy? org_two member CAN destroy their own time_reg on shared project" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + + assert policy_for(ron, @time_reg_ron).apply(:destroy?) + end + + # --- Scope (default) --- + + test "scope: org_two admin sees all time_regs on shared projects" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + result = scoped_time_regs(admin) + + # Should see time_regs from the shared project (project_1) + assert_includes result, @time_reg_joe + assert_includes result, @time_reg_ron + end + + test "scope: org_two member sees only their own time_regs" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + + result = scoped_time_regs(ron) + + # Ron should see his own time_regs + assert_includes result, @time_reg_ron + + # Ron should NOT see joe's time_regs + assert_not_includes result, @time_reg_joe + end +end From 4a37aac3425a9a6f483b134824f7a61ae8cc90bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:29:03 +0100 Subject: [PATCH 15/31] feat: extend TaskPolicy scope for shared project tasks Co-Authored-By: Claude Opus 4.6 (1M context) --- app/policies/task_policy.rb | 14 +++- test/policies/task_policy_test.rb | 109 ++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 test/policies/task_policy_test.rb diff --git a/app/policies/task_policy.rb b/app/policies/task_policy.rb index 553166a3..3798107d 100644 --- a/app/policies/task_policy.rb +++ b/app/policies/task_policy.rb @@ -14,12 +14,22 @@ def show? scope_for :relation do |relation| if user.organization_admin? - relation.where(organization: user.current_organization) + own_ids = relation.where(organization: user.current_organization).select(:id) + shared_project_ids = ProjectShare.where(organization_id: user.current_organization.id).select(:project_id) + shared_ids = relation.joins(:assigned_tasks).where(assigned_tasks: { project_id: shared_project_ids }).select(:id) + relation.where(id: own_ids).or(relation.where(id: shared_ids)).distinct elsif user.access_info.organization_spectator? projects = authorized_scope(Project.all, type: :relation).all relation.joins(:projects).where(projects: projects).distinct else - relation.joins(:users).where(organization: user.current_organization, users: { id: user.id }).distinct + own_ids = relation.joins(:users).where(organization: user.current_organization, users: { id: user.id }).select(:id) + user_shared_project_ids = ProjectAccess.joins(:access_info) + .where(access_infos: { user_id: user.id, organization_id: user.current_organization.id }) + .joins(:project) + .merge(Project.joins(:project_shares).where(project_shares: { organization_id: user.current_organization.id })) + .select(:project_id) + shared_ids = relation.joins(:assigned_tasks).where(assigned_tasks: { project_id: user_shared_project_ids }).select(:id) + relation.where(id: own_ids).or(relation.where(id: shared_ids)).distinct end end end diff --git a/test/policies/task_policy_test.rb b/test/policies/task_policy_test.rb new file mode 100644 index 00000000..0a1e25f0 --- /dev/null +++ b/test/policies/task_policy_test.rb @@ -0,0 +1,109 @@ +require "test_helper" + +class TaskPolicyTest < ActiveSupport::TestCase + fixtures :all + + setup do + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + + # Tasks belonging to org_one + @debug = tasks(:debug) # assigned to project_1 (shared with org_two) + @coding = tasks(:coding) # assigned to project_1 (shared with org_two) + @authentication = tasks(:authentication) # assigned to project_1 (shared with org_two) + @ux_audit = tasks(:ux_audit) # assigned to project_2 (NOT shared with org_two) + @e2e_testing = tasks(:e2e_testing) # org_one, no assigned_tasks on shared projects + + # Task belonging to org_two + @login = tasks(:login) + end + + # Helper to get the default scope result for a given user + def scoped_tasks(user) + policy = TaskPolicy.new(user: user) + policy.apply_scope(Task.all, type: :relation) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- Scope: org_two admin sees tasks assigned to shared projects --- + + test "scope: org_two admin sees tasks assigned to shared projects (org_one's tasks)" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + result = scoped_tasks(admin) + + # Should see org_two's own tasks + assert_includes result, @login + + # Should see org_one tasks assigned to project_1 (shared with org_two) + assert_includes result, @debug + assert_includes result, @coding + assert_includes result, @authentication + end + + test "scope: org_two admin does NOT see org_one tasks not assigned to shared projects" do + admin = users(:organization_admin) + switch_org_context!(admin, @org_two) + assert_equal @org_two, admin.current_organization + + result = scoped_tasks(admin) + + # ux_audit is assigned to project_2 which is NOT shared with org_two + assert_not_includes result, @ux_audit + + # e2e_testing belongs to org_one and has no assigned_tasks on shared projects + assert_not_includes result, @e2e_testing + end + + # --- Scope: org_two member with ProjectAccess sees tasks on shared projects --- + + test "scope: org_two member with ProjectAccess sees tasks on their shared projects" do + ron = users(:ron) + # ron is active in org_two by default and has ProjectAccess to project_1 via access_info_2 + assert_equal @org_two, ron.current_organization + + result = scoped_tasks(ron) + + # Should see tasks assigned to project_1 (shared project ron has access to) + assert_includes result, @debug + assert_includes result, @coding + assert_includes result, @authentication + end + + test "scope: org_two member does NOT see org_one tasks not on shared projects" do + ron = users(:ron) + assert_equal @org_two, ron.current_organization + + result = scoped_tasks(ron) + + # ux_audit is assigned to project_2 which is NOT shared with org_two + assert_not_includes result, @ux_audit + + # e2e_testing belongs to org_one, not assigned to any shared project + assert_not_includes result, @e2e_testing + end + + # --- Scope: org_one admin still sees all their own tasks --- + + test "scope: org_one admin sees all org_one tasks" do + admin = users(:organization_admin) + # admin is active in org_one by default + assert_equal @org_one, admin.current_organization + + result = scoped_tasks(admin) + + assert_includes result, @debug + assert_includes result, @coding + assert_includes result, @authentication + assert_includes result, @ux_audit + assert_includes result, @e2e_testing + assert_not_includes result, @login + end +end From deea32f907533c8cd8b9a16c03e81debd0976b3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:33:27 +0100 Subject: [PATCH 16/31] feat: add org-context-aware rate resolution to TimeReg Add used_rate_for(organization) and billed_amount_for(organization) methods that resolve rates based on the viewing organization's context. Guest orgs (with a ProjectShare) get their own rate hierarchy: task rate > project share rate > 0. The project owner org falls back to existing used_rate behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/models/time_reg.rb | 16 +++++++++++++++ test/models/time_reg_test.rb | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/app/models/time_reg.rb b/app/models/time_reg.rb index 86f57bbe..89cc078f 100644 --- a/app/models/time_reg.rb +++ b/app/models/time_reg.rb @@ -77,6 +77,17 @@ def used_rate assigned_task.rate.positive? ? assigned_task.rate : project.rate end + def used_rate_for(organization) + project_share = project.project_shares.find_by(organization: organization) + if project_share + task_rate = project_share.project_share_task_rates.find_by(assigned_task: assigned_task) + rate = task_rate&.rate || 0 + rate.positive? ? rate : project_share.rate + else + used_rate + end + end + def total_hours minutes.to_f / 60 # TODO: ensure the hours used in calculations is the same as hours displayed check `minutes_to_float` @@ -86,6 +97,11 @@ def billed_amount total_hours * used_rate end + def billed_amount_for(organization) + hours = minutes / 60.0 + hours * used_rate_for(organization) + end + protected def only_one_active_time_reg diff --git a/test/models/time_reg_test.rb b/test/models/time_reg_test.rb index f5a61e47..6b21ddb0 100644 --- a/test/models/time_reg_test.rb +++ b/test/models/time_reg_test.rb @@ -54,4 +54,43 @@ def setup @time_reg.save assert_not @time_reg.active? end + + # used_rate_for(organization) + + test "#used_rate_for returns same as used_rate when org is the project owner" do + owner_org = organizations(:organization_one) + assert_equal @time_reg.used_rate, @time_reg.used_rate_for(owner_org) + end + + test "#used_rate_for returns the task rate when guest org has a ProjectShareTaskRate" do + guest_org = organizations(:organization_two) + task_rate = project_share_task_rates(:task_rate_one) + assert_equal task_rate.rate, @time_reg.used_rate_for(guest_org) + end + + test "#used_rate_for returns the project share rate when guest org has no task rate" do + guest_org = organizations(:organization_two) + project_share = project_shares(:project_one_shared_with_org_two) + project_share.update!(rate: 200) + project_share.project_share_task_rates.destroy_all + + assert_equal 200, @time_reg.used_rate_for(guest_org) + end + + test "#used_rate_for returns 0 when guest org has no rates set" do + guest_org = organizations(:organization_two) + project_share = project_shares(:project_one_shared_with_org_two) + project_share.update!(rate: 0) + project_share.project_share_task_rates.destroy_all + + assert_equal 0, @time_reg.used_rate_for(guest_org) + end + + # billed_amount_for(organization) + + test "#billed_amount_for returns minutes / 60.0 * used_rate_for(organization)" do + guest_org = organizations(:organization_two) + expected = @time_reg.minutes / 60.0 * @time_reg.used_rate_for(guest_org) + assert_equal expected, @time_reg.billed_amount_for(guest_org) + end end From 51edfc992dec11807e068d1a80baaa108e18dfe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:40:46 +0100 Subject: [PATCH 17/31] feat: org-context-aware rate resolution in reports Reports::Summary and Reports::Result now accept an optional organization parameter. When provided, they use billed_amount_for(organization) to compute billable amounts using the guest org's ProjectShare rates instead of the owning org's rates. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/controllers/reports_controller.rb | 3 +- app/models/reports/result.rb | 13 ++++- app/models/reports/summary.rb | 9 ++- test/models/reports/result_test.rb | 81 +++++++++++++++++++++++++++ test/models/reports/summary_test.rb | 57 +++++++++++++++++++ 5 files changed, 158 insertions(+), 5 deletions(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index d293d8b5..1ff12928 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -7,8 +7,9 @@ class ReportsController < AuthenticatedController def index @summary = Reports::Summary.new( time_regs: @time_regs, + organization: current_user.current_organization, ) - @results = Reports::Result.new(time_regs: @time_regs, filter: @filter) + @results = Reports::Result.new(time_regs: @time_regs, filter: @filter, organization: current_user.current_organization) authorize! end diff --git a/app/models/reports/result.rb b/app/models/reports/result.rb index 5f0c52b6..4bd7e6c3 100644 --- a/app/models/reports/result.rb +++ b/app/models/reports/result.rb @@ -1,8 +1,9 @@ module Reports class Result - def initialize(time_regs:, filter:) + def initialize(time_regs:, filter:, organization: nil) @time_regs = time_regs @filter = filter + @organization = organization end def grouped @@ -39,12 +40,20 @@ def group_by(attribute:, attribute_name_method: :name) total_minutes: total_minutes, total_billable_minutes: total_billable_minutes, total_billable_minutes_percentage: total_billable_minutes_percentage, - total_billable_amount: ConvertCurrencyHundredths.out(billable_time_regs.sum(&:billed_amount)), + total_billable_amount: ConvertCurrencyHundredths.out(billable_amount(billable_time_regs)), group_link: { "#{singular_attribute}_ids": [ group.id ], category: nil } } end end + def billable_amount(time_regs) + if @organization + time_regs.sum { |tr| tr.billed_amount_for(@organization) } + else + time_regs.sum(&:billed_amount) + end + end + def project_billable?(time_reg) time_reg.project.billable end diff --git a/app/models/reports/summary.rb b/app/models/reports/summary.rb index 8e1b23c2..ec6f3403 100644 --- a/app/models/reports/summary.rb +++ b/app/models/reports/summary.rb @@ -1,11 +1,16 @@ module Reports class Summary - def initialize(time_regs:) + def initialize(time_regs:, organization: nil) @time_regs = time_regs + @organization = organization end def total_billable_amount - @total_billable_amount ||= @time_regs.billable.sum(&:billed_amount) + @total_billable_amount ||= if @organization + @time_regs.billable.sum { |tr| tr.billed_amount_for(@organization) } + else + @time_regs.billable.sum(&:billed_amount) + end end def total_billable_amount_currency diff --git a/test/models/reports/result_test.rb b/test/models/reports/result_test.rb index 8ca46fb3..0b841c76 100644 --- a/test/models/reports/result_test.rb +++ b/test/models/reports/result_test.rb @@ -68,6 +68,27 @@ def generate_expected_data(attribute:, attribute_name_method: :name) end end + def generate_expected_data_with_org(attribute:, organization:, attribute_name_method: :name) + singular_attribute = attribute.singularize.to_sym + grouped_time_regs = @time_regs.group_by(&singular_attribute) + grouped_time_regs.map do |group, time_regs| + billable_time_regs = time_regs.select { |time_reg| time_reg.project.billable } + total_minutes = time_regs.sum(&:minutes) + total_billable_minutes = billable_time_regs.sum(&:minutes) + total_billable_amount = ConvertCurrencyHundredths.out(billable_time_regs.sum { |tr| tr.billed_amount_for(organization) }) + total_billable_minutes_percentage = (total_billable_minutes / total_minutes.to_f * 100).truncate(2) + + { + attribute_name: group.send(attribute_name_method), + total_minutes: total_minutes, + total_billable_minutes: total_billable_minutes, + total_billable_amount: total_billable_amount, + total_billable_minutes_percentage: total_billable_minutes_percentage, + group_link: { "#{singular_attribute}_ids": [ group.id ], category: nil } + } + end + end + def generate_dummy_data # Organization @organization = Organization.create!(name: "My report test organization", currency: "DKK") @@ -149,4 +170,64 @@ def generate_dummy_data @time_reg16 = TimeReg.create!(user: @user2, assigned_task: @assigned_task1, minutes: 480, date_worked: Date.today - 8) end end + + class ResultWithOrganizationTest < ActiveSupport::TestCase + def setup + @owner_org = Organization.create!(name: "Result Owner Org", currency: "USD") + @guest_org = Organization.create!(name: "Result Guest Org", currency: "USD") + + @user = User.create!(first_name: "Result", last_name: "User", email: "result_org_test@example.com", password: "password") + AccessInfo.create!(user: @user, organization: @owner_org, role: 1) + + @client = Client.create!(name: "Result Org Client", organization: @owner_org) + + @task = Task.create!(name: "Result Org Task", organization: @owner_org) + + @project = Project.new(name: "Result Org Project", client: @client, rate: 10000, billable: true) + @assigned_task = AssignedTask.new(task: @task, project: @project, rate: 0) + @project.save!(validate: false) + @assigned_task.save!(validate: false) + + # Share project with guest org at a different rate + @project_share = ProjectShare.create!(project: @project, organization: @guest_org, rate: 5000) + + @time_reg = TimeReg.create!(user: @user, assigned_task: @assigned_task, minutes: 120, date_worked: Date.today) + @time_regs = [ @time_reg ] + + @filter_class = Reports::Filter + end + + test "grouped without organization uses default billed_amount" do + filter = @filter_class.new(category: @filter_class::CLIENTS) + result = Reports::Result.new(time_regs: @time_regs, filter: filter) + + grouped = result.grouped + # 120 min = 2 hours, rate 10000 => billed_amount = 20000 + expected_amount = ConvertCurrencyHundredths.out(@time_regs.select { |tr| tr.project.billable }.sum(&:billed_amount)) + assert_equal expected_amount, grouped.first[:total_billable_amount] + end + + test "grouped with organization uses billed_amount_for" do + filter = @filter_class.new(category: @filter_class::CLIENTS) + result = Reports::Result.new(time_regs: @time_regs, filter: filter, organization: @guest_org) + + grouped = result.grouped + # Guest org share rate = 5000, 2 hours * 5000 = 10000 + expected_amount = ConvertCurrencyHundredths.out(@time_regs.select { |tr| tr.project.billable }.sum { |tr| tr.billed_amount_for(@guest_org) }) + assert_equal expected_amount, grouped.first[:total_billable_amount] + + # Verify it differs from default + default_amount = ConvertCurrencyHundredths.out(@time_regs.select { |tr| tr.project.billable }.sum(&:billed_amount)) + assert_not_equal default_amount, grouped.first[:total_billable_amount] + end + + test "grouped by projects with organization uses org-context rates" do + filter = @filter_class.new(category: @filter_class::PROJECTS) + result = Reports::Result.new(time_regs: @time_regs, filter: filter, organization: @guest_org) + + grouped = result.grouped + expected_amount = ConvertCurrencyHundredths.out(@time_regs.select { |tr| tr.project.billable }.sum { |tr| tr.billed_amount_for(@guest_org) }) + assert_equal expected_amount, grouped.first[:total_billable_amount] + end + end end diff --git a/test/models/reports/summary_test.rb b/test/models/reports/summary_test.rb index b50de218..61e10ec4 100644 --- a/test/models/reports/summary_test.rb +++ b/test/models/reports/summary_test.rb @@ -27,4 +27,61 @@ def setup assert_equal @summary.total_minutes - @summary.total_billable_minutes, @summary.total_non_billable_minutes end end + + class SummaryWithOrganizationTest < ActiveSupport::TestCase + def setup + # Owner organization + @owner_org = Organization.create!(name: "Owner Org", currency: "USD") + + # Guest organization + @guest_org = Organization.create!(name: "Guest Org", currency: "USD") + + # User + @user = User.create!(first_name: "Test", last_name: "User", email: "summary_org_test@example.com", password: "password") + AccessInfo.create!(user: @user, organization: @owner_org, role: 1) + + # Client and project + @client = Client.create!(name: "Summary Org Client", organization: @owner_org) + + @project = Project.new(name: "Summary Org Project", client: @client, rate: 10000, billable: true) + @task = Task.create!(name: "Summary Org Task", organization: @owner_org) + @assigned_task = AssignedTask.new(task: @task, project: @project, rate: 0) + @project.save!(validate: false) + @assigned_task.save!(validate: false) + + # Create a ProjectShare with a different rate for the guest org + @project_share = ProjectShare.create!(project: @project, organization: @guest_org, rate: 5000) + + # Time registrations + @time_reg = TimeReg.create!(user: @user, assigned_task: @assigned_task, minutes: 60, date_worked: Date.today) + + @time_regs = TimeReg.where(id: @time_reg.id) + end + + test "#total_billable_amount without organization uses default billed_amount" do + summary = Reports::Summary.new(time_regs: @time_regs) + + # 60 minutes = 1 hour, rate = 10000 (project rate since assigned_task rate is 0) + expected = @time_regs.billable.sum(&:billed_amount) + assert_equal expected, summary.total_billable_amount + end + + test "#total_billable_amount with organization uses billed_amount_for" do + summary = Reports::Summary.new(time_regs: @time_regs, organization: @guest_org) + + # Guest org share rate = 5000, 1 hour * 5000 = 5000 + expected = @time_regs.billable.sum { |tr| tr.billed_amount_for(@guest_org) } + assert_equal expected, summary.total_billable_amount + # Verify it differs from the default + default_amount = @time_regs.billable.sum(&:billed_amount) + assert_not_equal default_amount, summary.total_billable_amount + end + + test "#total_billable_amount_currency with organization uses org-context rates" do + summary = Reports::Summary.new(time_regs: @time_regs, organization: @guest_org) + + expected = ConvertCurrencyHundredths.out(@time_regs.billable.sum { |tr| tr.billed_amount_for(@guest_org) }) + assert_equal expected, summary.total_billable_amount_currency + end + end end From 4c8f67cda0696263f96f58529f93e0b2a6feb6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:42:19 +0100 Subject: [PATCH 18/31] feat: add project_shares routes --- config/routes.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index 97c75442..15300234 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -60,6 +60,12 @@ namespace :workspace do resources :projects do post :import_modal, on: :collection + resources :project_shares, only: [ :index, :destroy ] do + member do + patch :update_rates + end + resources :project_share_task_rates, only: [ :create, :update ] + end end scope module: :projects do From 82321ae73df9208018a414b4f7638a60ad653df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 15:48:01 +0100 Subject: [PATCH 19/31] feat: add DisconnectProjectShareService Co-Authored-By: Claude Opus 4.6 (1M context) --- .../disconnect_project_share_service.rb | 46 +++++++ .../disconnect_project_share_service_test.rb | 119 ++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 app/services/disconnect_project_share_service.rb create mode 100644 test/services/disconnect_project_share_service_test.rb diff --git a/app/services/disconnect_project_share_service.rb b/app/services/disconnect_project_share_service.rb new file mode 100644 index 00000000..507a414e --- /dev/null +++ b/app/services/disconnect_project_share_service.rb @@ -0,0 +1,46 @@ +class DisconnectProjectShareService + attr_reader :project_share + + def initialize(project_share:) + @project_share = project_share + end + + def call + ActiveRecord::Base.transaction do + destroy_guest_project_accesses + cancel_pending_invitations + project_share.destroy! + end + end + + private + + def project + project_share.project + end + + def guest_organization + project_share.organization + end + + def guest_access_infos + guest_organization.access_infos + end + + def destroy_guest_project_accesses + ProjectAccess.where( + project: project, + access_info: guest_access_infos + ).destroy_all + end + + def cancel_pending_invitations + guest_emails = guest_organization.users.pluck(:email) + return if guest_emails.empty? + + ProjectInvitation + .pending + .where(project: project, invited_email: guest_emails) + .find_each(&:reject!) + end +end diff --git a/test/services/disconnect_project_share_service_test.rb b/test/services/disconnect_project_share_service_test.rb new file mode 100644 index 00000000..b1e1bdb0 --- /dev/null +++ b/test/services/disconnect_project_share_service_test.rb @@ -0,0 +1,119 @@ +require "test_helper" + +class DisconnectProjectShareServiceTest < ActiveSupport::TestCase + def setup + @project_share = project_shares(:project_one_shared_with_org_two) + @project = projects(:project_1) + @guest_org = organizations(:organization_two) + end + + test "destroys the ProjectShare record" do + assert_difference "ProjectShare.count", -1 do + DisconnectProjectShareService.new(project_share: @project_share).call + end + + assert_nil ProjectShare.find_by(id: @project_share.id) + end + + test "cascades to destroy ProjectShareTaskRate records" do + assert project_share_task_rates(:task_rate_one).present? + + assert_difference "ProjectShareTaskRate.count", -1 do + DisconnectProjectShareService.new(project_share: @project_share).call + end + end + + test "destroys all ProjectAccess records for guest org users on this project" do + # ron_org2_shared_project_1 is ron's org_two access to project_1 + ron_org2_access = project_accesses(:ron_org2_shared_project_1) + assert ron_org2_access.present? + + DisconnectProjectShareService.new(project_share: @project_share).call + + assert_nil ProjectAccess.find_by(id: ron_org2_access.id) + end + + test "does not destroy ProjectAccess records for the owning org users" do + # ron_project_1 is ron's org_one access to project_1 (via access_info_3) + ron_org1_access = project_accesses(:ron_project_1) + spectator_access = project_accesses(:spectator_project_1) + + DisconnectProjectShareService.new(project_share: @project_share).call + + assert ProjectAccess.find_by(id: ron_org1_access.id), "Ron's org_one access should be preserved" + assert ProjectAccess.find_by(id: spectator_access.id), "Spectator's access should be preserved" + end + + test "cancels pending ProjectInvitation records for known guest org users" do + # Create a user who belongs only to org_two (not org_one, which owns the project) + guest_only_user = User.create!( + email: "guest_only@example.com", + first_name: "Guest", + last_name: "Only", + password: "password123", + locale: "en" + ) + AccessInfo.create!( + user: guest_only_user, + organization: @guest_org, + role: :organization_user + ) + + # Create a pending invitation for this guest-org-only user + invitation = ProjectInvitation.create!( + project: @project, + invited_email: guest_only_user.email, + invited_by: users(:organization_admin), + invited_at: Time.current + ) + + assert invitation.pending? + + DisconnectProjectShareService.new(project_share: @project_share).call + + invitation.reload + assert invitation.rejected?, "Pending invitation for guest org user should be cancelled" + end + + test "does not cancel invitations for users not in the guest org" do + # Create a pending invitation for someone not in org_two + invitation = ProjectInvitation.create!( + project: @project, + invited_email: "outsider@example.com", + invited_by: users(:organization_admin), + invited_at: Time.current + ) + + DisconnectProjectShareService.new(project_share: @project_share).call + + invitation.reload + assert invitation.pending?, "Invitation for non-guest-org user should remain pending" + end + + test "preserves TimeReg records" do + time_reg_count = TimeReg.count + + DisconnectProjectShareService.new(project_share: @project_share).call + + assert_equal time_reg_count, TimeReg.count, "TimeReg records should be preserved" + end + + test "works when called by org_one admin disconnecting a guest" do + # This simulates org_one (project owner) disconnecting org_two + assert_nothing_raised do + DisconnectProjectShareService.new(project_share: @project_share).call + end + + assert_nil ProjectShare.find_by(id: @project_share.id) + end + + test "works when called by org_two admin disconnecting self" do + # This simulates org_two (guest) disconnecting themselves + # The service should work the same regardless of who initiates it + assert_nothing_raised do + DisconnectProjectShareService.new(project_share: @project_share).call + end + + assert_nil ProjectShare.find_by(id: @project_share.id) + end +end From 0a3020ba3e935523ab83a188ada37232b370fba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:01:32 +0100 Subject: [PATCH 20/31] feat: add Workspace::ProjectSharesController Implements controller for managing project shares with index, update_rates, and destroy actions. Adds accepts_nested_attributes_for to ProjectShare model to support bulk rate updates for project share task rates. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../workspace/project_shares_controller.rb | 54 ++++++++ app/models/project_share.rb | 1 + .../project_shares_controller_test.rb | 117 ++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 app/controllers/workspace/project_shares_controller.rb create mode 100644 test/controllers/workspace/project_shares_controller_test.rb diff --git a/app/controllers/workspace/project_shares_controller.rb b/app/controllers/workspace/project_shares_controller.rb new file mode 100644 index 00000000..b22a6f69 --- /dev/null +++ b/app/controllers/workspace/project_shares_controller.rb @@ -0,0 +1,54 @@ +module Workspace + class ProjectSharesController < WorkspaceController + before_action :set_project + before_action :set_project_share, only: %i[update_rates destroy] + + def index + authorize! ProjectShare + @project_shares = authorized_scope(ProjectShare, type: :relation) + .where(project: @project) + render json: @project_shares + end + + def update_rates + authorize! @project_share, to: :update? + if @project_share.update(project_share_params) + redirect_to workspace_project_project_shares_path(@project) + else + redirect_to workspace_project_project_shares_path(@project), alert: t("alert.unable_to_proceed") + end + end + + def destroy + authorize! @project_share + DisconnectProjectShareService.new(project_share: @project_share).call + + if owner_org? + redirect_to workspace_project_path(@project), notice: t("notice.project_share_disconnected") + else + redirect_to workspace_projects_path, notice: t("notice.project_share_disconnected") + end + end + + private + + def set_project + @project = authorized_scope(Project, type: :relation).find(params[:project_id]) + end + + def set_project_share + @project_share = authorized_scope(ProjectShare, type: :relation).find(params[:id]) + end + + def project_share_params + params.require(:project_share).permit( + :rate_currency, + project_share_task_rates_attributes: [ :id, :rate_currency ] + ) + end + + def owner_org? + @project.organization == current_user.current_organization + end + end +end diff --git a/app/models/project_share.rb b/app/models/project_share.rb index 367d7ffc..bb26c4d5 100644 --- a/app/models/project_share.rb +++ b/app/models/project_share.rb @@ -5,6 +5,7 @@ class ProjectShare < ApplicationRecord belongs_to :organization has_many :project_share_task_rates, dependent: :destroy + accepts_nested_attributes_for :project_share_task_rates validates :organization_id, uniqueness: { scope: :project_id } validate :organization_is_not_project_owner diff --git a/test/controllers/workspace/project_shares_controller_test.rb b/test/controllers/workspace/project_shares_controller_test.rb new file mode 100644 index 00000000..209a4638 --- /dev/null +++ b/test/controllers/workspace/project_shares_controller_test.rb @@ -0,0 +1,117 @@ +require "test_helper" + +module Workspace + class ProjectSharesControllerTest < ActionController::TestCase + fixtures :all + + setup do + @organization_admin = users(:organization_admin) + sign_in @organization_admin + + @project = projects(:project_1) + @project_share = project_shares(:project_one_shared_with_org_two) + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- index --- + + test "index as org_one admin lists guest orgs for the project" do + get :index, params: { project_id: @project.id } + assert_response :success + end + + test "index as org_two admin lists shared projects" do + switch_org_context!(@organization_admin, @org_two) + get :index, params: { project_id: @project.id } + assert_response :success + end + + test "index calls authorize!" do + assert_authorized_to(:index?, ProjectShare, with: Workspace::ProjectSharePolicy) do + get :index, params: { project_id: @project.id } + end + end + + # --- update_rates --- + + test "update_rates as org_two admin updates project_share rate and task rates" do + switch_org_context!(@organization_admin, @org_two) + sign_in @organization_admin + task_rate = project_share_task_rates(:task_rate_one) + + patch :update_rates, params: { + project_id: @project.id, + id: @project_share.id, + project_share: { + rate_currency: "5.00", + project_share_task_rates_attributes: { + "0" => { id: task_rate.id, rate_currency: "3.00" } + } + } + } + + assert_response :redirect + assert_equal 500, @project_share.reload.rate + assert_equal 300, task_rate.reload.rate + end + + test "update_rates as org_one admin denied" do + patch :update_rates, params: { + project_id: @project.id, + id: @project_share.id, + project_share: { rate_currency: "5.00" } + } + + # org_one admin cannot update guest rates (update? policy denies) + assert_redirected_to root_path + end + + test "update_rates calls authorize!" do + switch_org_context!(@organization_admin, @org_two) + assert_authorized_to(:update?, @project_share, with: Workspace::ProjectSharePolicy) do + patch :update_rates, params: { + project_id: @project.id, + id: @project_share.id, + project_share: { rate_currency: "5.00" } + } + end + end + + # --- destroy --- + + test "destroy as org_one admin disconnects guest org" do + assert_difference("ProjectShare.count", -1) do + delete :destroy, params: { project_id: @project.id, id: @project_share.id } + end + assert_redirected_to workspace_project_path(@project) + end + + test "destroy as org_two admin disconnects own org" do + switch_org_context!(@organization_admin, @org_two) + + assert_difference("ProjectShare.count", -1) do + delete :destroy, params: { project_id: @project.id, id: @project_share.id } + end + assert_redirected_to workspace_projects_path + end + + test "destroy as non-admin denied" do + sign_in users(:joe) + delete :destroy, params: { project_id: @project.id, id: @project_share.id } + assert_redirected_to root_path + end + + test "destroy calls authorize!" do + assert_authorized_to(:destroy?, @project_share, with: Workspace::ProjectSharePolicy) do + delete :destroy, params: { project_id: @project.id, id: @project_share.id } + end + end + end +end From 5335aafa5a2f1f3fb5de1ceb1c263240a970bf3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:09:37 +0100 Subject: [PATCH 21/31] feat: add Workspace::ProjectShareTaskRatesController Co-Authored-By: Claude Opus 4.6 (1M context) --- .../project_share_task_rates_controller.rb | 46 +++++++ ...roject_share_task_rates_controller_test.rb | 127 ++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 app/controllers/workspace/project_share_task_rates_controller.rb create mode 100644 test/controllers/workspace/project_share_task_rates_controller_test.rb diff --git a/app/controllers/workspace/project_share_task_rates_controller.rb b/app/controllers/workspace/project_share_task_rates_controller.rb new file mode 100644 index 00000000..62323230 --- /dev/null +++ b/app/controllers/workspace/project_share_task_rates_controller.rb @@ -0,0 +1,46 @@ +module Workspace + class ProjectShareTaskRatesController < WorkspaceController + before_action :set_project + before_action :set_project_share + before_action :set_task_rate, only: :update + + def create + @task_rate = @project_share.project_share_task_rates.new(task_rate_params) + authorize! @task_rate + + if @task_rate.save + redirect_to workspace_project_path(@project), notice: t("notice.rates_updated") + else + redirect_to workspace_project_path(@project), alert: t("alert.unable_to_proceed") + end + end + + def update + authorize! @task_rate + + if @task_rate.update(task_rate_params) + redirect_to workspace_project_path(@project), notice: t("notice.rates_updated") + else + redirect_to workspace_project_path(@project), alert: t("alert.unable_to_proceed") + end + end + + private + + def set_project + @project = authorized_scope(Project, type: :relation).find(params[:project_id]) + end + + def set_project_share + @project_share = authorized_scope(ProjectShare, type: :relation).find(params[:project_share_id]) + end + + def set_task_rate + @task_rate = authorized_scope(ProjectShareTaskRate, type: :relation).find(params[:id]) + end + + def task_rate_params + params.require(:project_share_task_rate).permit(:assigned_task_id, :rate_currency) + end + end +end diff --git a/test/controllers/workspace/project_share_task_rates_controller_test.rb b/test/controllers/workspace/project_share_task_rates_controller_test.rb new file mode 100644 index 00000000..84ec28fd --- /dev/null +++ b/test/controllers/workspace/project_share_task_rates_controller_test.rb @@ -0,0 +1,127 @@ +require "test_helper" + +module Workspace + class ProjectShareTaskRatesControllerTest < ActionController::TestCase + fixtures :all + + setup do + @organization_admin = users(:organization_admin) + sign_in @organization_admin + + @project = projects(:project_1) + @project_share = project_shares(:project_one_shared_with_org_two) + @task_rate = project_share_task_rates(:task_rate_one) + @org_one = organizations(:organization_one) + @org_two = organizations(:organization_two) + end + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + # --- create --- + + test "create as org_two admin creates a task rate on the project share" do + switch_org_context!(@organization_admin, @org_two) + sign_in @organization_admin + assigned_task = assigned_task(:task_2) + + assert_difference("ProjectShareTaskRate.count", 1) do + post :create, params: { + project_id: @project.id, + project_share_id: @project_share.id, + project_share_task_rate: { + assigned_task_id: assigned_task.id, + rate_currency: "4.50" + } + } + end + + assert_response :redirect + new_rate = ProjectShareTaskRate.last + assert_equal @project_share.id, new_rate.project_share_id + assert_equal assigned_task.id, new_rate.assigned_task_id + assert_equal 450, new_rate.rate + end + + test "create as org_one admin denied" do + post :create, params: { + project_id: @project.id, + project_share_id: @project_share.id, + project_share_task_rate: { + assigned_task_id: assigned_task(:task_2).id, + rate_currency: "4.50" + } + } + + assert_redirected_to root_path + end + + test "create calls authorize!" do + switch_org_context!(@organization_admin, @org_two) + sign_in @organization_admin + + # verify_authorized ensures authorize! is called; this would raise + # ActionPolicy::UnauthorizedError if authorize! were missing + post :create, params: { + project_id: @project.id, + project_share_id: @project_share.id, + project_share_task_rate: { + assigned_task_id: assigned_task(:task_2).id, + rate_currency: "4.50" + } + } + assert_response :redirect + end + + # --- update --- + + test "update as org_two admin updates an existing task rate" do + switch_org_context!(@organization_admin, @org_two) + sign_in @organization_admin + + patch :update, params: { + project_id: @project.id, + project_share_id: @project_share.id, + id: @task_rate.id, + project_share_task_rate: { + rate_currency: "7.50" + } + } + + assert_response :redirect + assert_equal 750, @task_rate.reload.rate + end + + test "update as org_one admin denied" do + patch :update, params: { + project_id: @project.id, + project_share_id: @project_share.id, + id: @task_rate.id, + project_share_task_rate: { + rate_currency: "7.50" + } + } + + assert_redirected_to root_path + end + + test "update calls authorize!" do + switch_org_context!(@organization_admin, @org_two) + sign_in @organization_admin + + assert_authorized_to(:update?, @task_rate, with: Workspace::ProjectShareTaskRatePolicy) do + patch :update, params: { + project_id: @project.id, + project_share_id: @project_share.id, + id: @task_rate.id, + project_share_task_rate: { + rate_currency: "7.50" + } + } + end + end + end +end From 0d7c42471f5bb1afacb5fcd98ee0addc343f9c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:15:07 +0100 Subject: [PATCH 22/31] feat: update ProjectsController for shared project context Add shared project context variables to the show action: - @shared_project and @project_share for guest org admins viewing shared projects - @guest_organizations for owning org admins to see which orgs have access Add tests verifying guest org admin can view but not edit/update/destroy shared projects. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../workspace/projects_controller.rb | 7 ++ .../workspace/projects_controller_test.rb | 64 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/app/controllers/workspace/projects_controller.rb b/app/controllers/workspace/projects_controller.rb index 4edf5b0a..e7b4cae4 100644 --- a/app/controllers/workspace/projects_controller.rb +++ b/app/controllers/workspace/projects_controller.rb @@ -29,6 +29,13 @@ def create def show authorize! @project + + @shared_project = @project.project_shares.exists?(organization: current_user.current_organization) + @project_share = @project.project_shares.find_by(organization: current_user.current_organization) + + unless @shared_project + @guest_organizations = @project.project_shares.includes(:organization) + end end def update diff --git a/test/controllers/workspace/projects_controller_test.rb b/test/controllers/workspace/projects_controller_test.rb index b54c67e0..ee607f3c 100644 --- a/test/controllers/workspace/projects_controller_test.rb +++ b/test/controllers/workspace/projects_controller_test.rb @@ -90,5 +90,69 @@ class ProjectsControllerTest < ActionController::TestCase end assert_redirected_to workspace_project_path(@project) end + + # --- Shared project tests (guest org admin) --- + + # Helper to switch a user's active context to a given organization + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + + test "org_two admin can access show for a shared project" do + shared_project = projects(:project_1) + switch_org_context!(@organization_admin, organizations(:organization_two)) + sign_in @organization_admin + + get :show, params: { id: shared_project.id } + assert_response :success + assert assigns(:shared_project), "Expected @shared_project to be set for guest org admin" + assert_not_nil assigns(:project_share), "Expected @project_share to be set for guest org admin" + end + + test "org_one admin sees guest_organizations for owned shared project" do + get :show, params: { id: @project.id } + assert_response :success + assert_not assigns(:shared_project), "Expected @shared_project to be false for owning org admin" + assert_not_nil assigns(:guest_organizations), "Expected @guest_organizations to be set for owning org admin" + end + + test "org_two admin sees shared projects in index" do + switch_org_context!(@organization_admin, organizations(:organization_two)) + sign_in @organization_admin + + get :index + assert_response :success + end + + test "org_two admin cannot access edit for shared project" do + shared_project = projects(:project_1) + switch_org_context!(@organization_admin, organizations(:organization_two)) + sign_in @organization_admin + + get :edit, params: { id: shared_project.id } + assert_redirected_to root_path + end + + test "org_two admin cannot access update for shared project" do + shared_project = projects(:project_1) + switch_org_context!(@organization_admin, organizations(:organization_two)) + sign_in @organization_admin + + patch :update, params: { id: shared_project.id, project: { name: "Hacked Name" } } + assert_redirected_to root_path + assert_equal "E Corp CRM", shared_project.reload.name + end + + test "org_two admin cannot access destroy for shared project" do + shared_project = projects(:project_1) + switch_org_context!(@organization_admin, organizations(:organization_two)) + sign_in @organization_admin + + assert_no_difference("Project.count") do + delete :destroy, params: { id: shared_project.id } + end + assert_redirected_to root_path + end end end From ad9215bc2b616ba32682425d0b776d66e80e66ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:26:26 +0100 Subject: [PATCH 23/31] feat: update TimeRegsController for shared projects - Update ProjectPolicy scopes to include shared projects via ProjectShare - Add authorize! to export action (was missing) - Move export? to class-based authorization in TimeRegPolicy - Strip cross-org user emails in CSV export - Add tests for org_two member creating time_regs on shared projects - Add tests for task dropdown loading shared project tasks - Add tests for admin export with email stripping Co-Authored-By: Claude Opus 4.6 (1M context) --- app/controllers/time_regs_controller.rb | 6 +- app/policies/project_policy.rb | 36 +++++-- app/policies/time_reg_policy.rb | 4 +- test/controllers/time_regs_controller_test.rb | 96 +++++++++++++++++++ 4 files changed, 133 insertions(+), 9 deletions(-) diff --git a/app/controllers/time_regs_controller.rb b/app/controllers/time_regs_controller.rb index 837cc06b..8eb1fc8d 100644 --- a/app/controllers/time_regs_controller.rb +++ b/app/controllers/time_regs_controller.rb @@ -93,8 +93,10 @@ def toggle_active # exports the time_regs in a project to a .CSV def export + authorize! project = authorized_scope(Project, type: :relation).find(params[:project_id]) client = project.client + current_org = current_user.current_organization time_regs = project.time_regs.includes( :task, :user, @@ -106,8 +108,10 @@ def export csv << [ "date", "client", "project", "task", "notes", "minutes", "first name", "last name", "email" ] # Add CSV data rows for each time_reg time_regs.each do |time_reg| + # Strip email for users not in the current organization + email = time_reg.user.access_infos.exists?(organization: current_org) ? time_reg.user.email : "" csv << [ time_reg.date_worked, client.name, project.name, time_reg.assigned_task.task.name, - time_reg.notes, time_reg.minutes, time_reg.user.first_name, time_reg.user.last_name, time_reg.user.email ] + time_reg.notes, time_reg.minutes, time_reg.user.first_name, time_reg.user.last_name, email ] end end send_data csv_data, filename: "#{Time.now.to_i}_time_regs_for_#{project.name}.csv" diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index a78748bf..22414cde 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -17,19 +17,43 @@ def invite_external_user? scope_for :relation, :own do |relation| organization = user.current_organization - relation = relation.joins(client: :organization).where(organizations: { id: organization.id }) + + own_ids = relation.joins(client: :organization) + .where(organizations: { id: organization.id }) + .select(:id) + shared_ids = relation.joins(:project_shares) + .where(project_shares: { organization_id: organization.id }) + .select(:id) + combined = relation.where(id: own_ids).or(relation.where(id: shared_ids)) + unless user.organization_admin? - relation = relation.joins(:project_accesses).where(project_accesses: user.project_accesses) if user.project_restricted?(organization) + combined = combined.where( + id: ProjectAccess.where(access_info: user.access_info(organization)) + .select(:project_id) + ) if user.project_restricted?(organization) end - relation.distinct + + combined.distinct end scope_for :relation do |relation| organization = user.current_organization - relation = relation.joins(client: :organization).where(organizations: { id: organization.id }) + + own_ids = relation.joins(client: :organization) + .where(organizations: { id: organization.id }) + .select(:id) + shared_ids = relation.joins(:project_shares) + .where(project_shares: { organization_id: organization.id }) + .select(:id) + combined = relation.where(id: own_ids).or(relation.where(id: shared_ids)) + unless user.organization_admin? - relation = relation.joins(:project_accesses).where(project_accesses: user.project_accesses) if user.project_restricted?(organization) + combined = combined.where( + id: ProjectAccess.where(access_info: user.access_info(organization)) + .select(:project_id) + ) if user.project_restricted?(organization) end - relation.distinct + + combined.distinct end end diff --git a/app/policies/time_reg_policy.rb b/app/policies/time_reg_policy.rb index e72c48d9..24277a7f 100644 --- a/app/policies/time_reg_policy.rb +++ b/app/policies/time_reg_policy.rb @@ -4,14 +4,14 @@ def show? user == record.user || is_admin_allowed || admin_of_shared_project? end - %i[ edit update destroy toggle_active export ].each do |action| + %i[ edit update destroy toggle_active ].each do |action| define_method("#{action}?") do is_admin_allowed = user.organization_admin? && record.organization == user.current_organization user == record.user || is_admin_allowed end end - %i[ index new_modal edit_modal update_tasks_select ].each do |action| + %i[ index new_modal edit_modal update_tasks_select export ].each do |action| define_method("#{action}?") do !user.access_info.organization_spectator? end diff --git a/test/controllers/time_regs_controller_test.rb b/test/controllers/time_regs_controller_test.rb index 7ad4ee25..28da00dd 100644 --- a/test/controllers/time_regs_controller_test.rb +++ b/test/controllers/time_regs_controller_test.rb @@ -164,4 +164,100 @@ class AdminUser < TimeRegsControllerTest end end end + + class SharedProjectMember < TimeRegsControllerTest + setup do + @ron = users(:ron) + @org_two = organizations(:organization_two) + @shared_project = projects(:project_1) + @current_date = Date.today + + # Switch ron to org_two context + switch_org_context!(@ron, @org_two) + sign_in @ron + end + + test "org_two member can create a time_reg on a shared project" do + assigned_task = @shared_project.assigned_tasks.first + assert_difference("TimeReg.count") do + post :create, params: { time_reg: { date_worked: @current_date, minutes: 60, project_id: @shared_project.id, assigned_task_id: assigned_task.id } } + end + assert_redirected_to time_regs_path(date: @current_date) + assert_equal @ron, TimeReg.last.user + assert_equal assigned_task, TimeReg.last.assigned_task + end + + test "task dropdown loads org_one tasks for the shared project" do + get :update_tasks_select, params: { project_id: @shared_project.id } + assert_response :success + + # Should include active tasks from project_1 (debug, coding, authentication) + active_task_ids = @shared_project.assigned_tasks.where(is_archived: false).pluck(:id) + returned_ids = assigns(:name_id_pairs).map(&:last) + active_task_ids.each do |task_id| + assert_includes returned_ids, task_id + end + end + + test "shared project appears in set_clients for org_two member" do + get :index + assert_response :success + project_names = assigns(:clients).flat_map { |c| c.items.map(&:name) } + assert_includes project_names, @shared_project.name + end + + private + + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + end + + class SharedProjectAdmin < TimeRegsControllerTest + setup do + @admin = users(:organization_admin) + @org_two = organizations(:organization_two) + @shared_project = projects(:project_1) + @current_date = Date.today + + # Switch admin to org_two context + switch_org_context!(@admin, @org_two) + sign_in @admin + end + + test "org_two admin can view time_regs on shared projects via index" do + get :index + assert_response :success + end + + test "org_two admin can export shared project time_regs" do + get :export, params: { project_id: @shared_project.id } + assert_response :success + assert_equal "text/csv", response.media_type + end + + test "export strips email for cross-org users on shared project" do + get :export, params: { project_id: @shared_project.id } + csv_lines = response.body.split("\n") + # Header has "email" column + header = csv_lines.first + assert_includes header, "email" + + # Find rows for org_one users (e.g., joe) - their emails should be redacted + joe = users(:joe) + csv_lines[1..].each do |line| + fields = CSV.parse_line(line) + next unless fields[6] == joe.first_name && fields[7] == joe.last_name + assert_equal "", fields[8], "Cross-org user email should be blank" + end + end + + private + + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end + end end From dc9512a68a1f1f9f5bbcd009ca9a9ac900e029fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:32:14 +0100 Subject: [PATCH 24/31] Add shared project indicator to project list Show "Shared by [org_name]" badge on projects shared with the current organization. Hide edit/delete buttons for shared projects since guest org admins cannot modify them. Add shared_with? helper to Project model and preload project_shares in index to avoid N+1 queries. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../workspace/projects_controller.rb | 2 +- app/models/project.rb | 12 +++++++++ .../workspace/projects/_project.html.erb | 26 ++++++++++++------- config/locales/en.yml | 11 ++++++++ config/locales/nb.yml | 11 ++++++++ 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/app/controllers/workspace/projects_controller.rb b/app/controllers/workspace/projects_controller.rb index e7b4cae4..6ab19aaf 100644 --- a/app/controllers/workspace/projects_controller.rb +++ b/app/controllers/workspace/projects_controller.rb @@ -50,7 +50,7 @@ def update end def index - @pagy, @clients = pagy authorized_scope(Client, type: :relation).order(:name).includes(:projects), items: 6 + @pagy, @clients = pagy authorized_scope(Client, type: :relation).order(:name).includes(projects: :project_shares), items: 6 authorize! end diff --git a/app/models/project.rb b/app/models/project.rb index b2e5cf60..922344b8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -29,6 +29,18 @@ def onboarding? @onboarding end + def shared_with?(organization) + if project_shares.loaded? + project_shares.any? { |ps| ps.organization_id == organization.id } + else + project_shares.exists?(organization: organization) + end + end + + def owning_organization + organization + end + def must_have_at_least_one_active_assigned_task errors.add(:tasks, :blank) if assigned_tasks.to_a.reject { |assigned_task| assigned_task.marked_for_destruction? || assigned_task.is_archived }.empty? end diff --git a/app/views/workspace/projects/_project.html.erb b/app/views/workspace/projects/_project.html.erb index fbb540c3..92a40348 100644 --- a/app/views/workspace/projects/_project.html.erb +++ b/app/views/workspace/projects/_project.html.erb @@ -1,8 +1,14 @@ +<% shared = project.shared_with?(current_user.current_organization) %> <%= render RubyUI::TableRow.new(id: "#{dom_id(project)}_listitem") do %> <%= render RubyUI::TableCell.new do %> <%= link_to workspace_project_path(project), class: "flex flex-col pl-2" do %>
<%= project.name %> + <% if shared %> + <%= render RubyUI::Badge.new(variant: :purple, class: "!rounded-full flex gap-x-1") do %> + <%= t("common.shared_by", org_name: project.owning_organization.name) %> + <% end %> + <% end %>
<%= project.description %> @@ -24,17 +30,19 @@ <% end %> <%= render RubyUI::TableCell.new(class: "flex align-center justify-end gap-2 h-full") do %> - <%= render TooltipComponent.new(note: t("common.update")) do %> - <%= render ButtonComponent.new(path: edit_workspace_project_path(project), method: :get, icon: true, variant: :outline) do %> - + <% unless shared %> + <%= render TooltipComponent.new(note: t("common.update")) do %> + <%= render ButtonComponent.new(path: edit_workspace_project_path(project), method: :get, icon: true, variant: :outline) do %> + + <% end %> <% end %> - <% end %> - <%- has_time_regs = project.time_regs.any? %> - <%- turbo_body = has_time_regs ? "The project #{project.name} has time registrations and can not be deleted." : "#{t("notice.deletion_of_he_project")} #{project.name}, #{t("notice.cannot_be_undone")}" %> - <%= render TooltipComponent.new(note: has_time_regs ? t("common.project_has_time_regs") : t("common.delete")) do %> - <%= render ButtonComponent.new(variant: :outline, method: :delete, icon: true, path: workspace_project_path(project), disabled: has_time_regs, form: { data: { turbo_confirm: turbo_body } }) do %> - + <%- has_time_regs = project.time_regs.any? %> + <%- turbo_body = has_time_regs ? "The project #{project.name} has time registrations and can not be deleted." : "#{t("notice.deletion_of_he_project")} #{project.name}, #{t("notice.cannot_be_undone")}" %> + <%= render TooltipComponent.new(note: has_time_regs ? t("common.project_has_time_regs") : t("common.delete")) do %> + <%= render ButtonComponent.new(variant: :outline, method: :delete, icon: true, path: workspace_project_path(project), disabled: has_time_regs, form: { data: { turbo_confirm: turbo_body } }) do %> + + <% end %> <% end %> <% end %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 70ae62ff..746a2b41 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -251,6 +251,15 @@ en: organization_settings: "Organization settings" settings: "Settings" currency: "Currency" + shared_by: "Shared by %{org_name}" + shared_with: "Shared with" + shared_project: "Shared project" + disconnect: "Disconnect" + owned_by: "Owned by %{org_name}" + project_rate: "Project Rate" + task_rates: "Task Rates" + update_rates: "Update Rates" + no_shared_organizations: "This project is not shared with any organizations." empty: title: "Oops, Nothing Here Yet!" subtext: "There's currently nothing to see here yet." @@ -327,6 +336,8 @@ en: removal_of_the_task: "Do you want to proceed with the removal of the task" cannot_be_undone: "from this project, This action cannot be undone." deletion_of_he_project: "Do you want to proceed with the deletion of the project" + project_share_disconnected: "Project share has been disconnected." + project_share_rates_updated: "Project share rates have been updated." alert: does_not_exist: "does not exist" diff --git a/config/locales/nb.yml b/config/locales/nb.yml index 1440d592..4c8e4657 100644 --- a/config/locales/nb.yml +++ b/config/locales/nb.yml @@ -268,6 +268,15 @@ nb: organization_settings: "Organisasjonsinnstillinger" settings: "Innstillinger" currency: "Valuta" + shared_by: "Delt av %{org_name}" + shared_with: "Delt med" + shared_project: "Delt prosjekt" + disconnect: "Koble fra" + owned_by: "Eid av %{org_name}" + project_rate: "Prosjekttimepris" + task_rates: "Oppgavepriser" + update_rates: "Oppdater priser" + no_shared_organizations: "Dette prosjektet er ikke delt med noen organisasjoner." empty: title: "Oops, ingenting her ennå!" subtext: "Det er foreløpig ingenting å se her ennå." @@ -344,6 +353,8 @@ nb: removal_of_the_task: "Vil du fortsette med fjerningen av oppgaven" cannot_be_undone: "fra dette prosjektet, denne handlingen kan ikke angres." deletion_of_he_project: "Vil du fortsette med slettingen av prosjektet" + project_share_disconnected: "Prosjektdelingen har blitt frakoblet." + project_share_rates_updated: "Prosjektdelingspriser har blitt oppdatert." alert: From 30e6ab5e33011b0b1ecafd979a3097cefb876288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:32:55 +0100 Subject: [PATCH 25/31] Add "Shared with" section to project show page Create _shared_organizations partial showing guest organizations with disconnect buttons. Only visible to owning org admins when @guest_organizations is present. Each org entry has a disconnect button that triggers the ProjectSharesController#destroy action. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../projects/_shared_organizations.html.erb | 24 +++++++++++++++++++ app/views/workspace/projects/show.html.erb | 3 +++ 2 files changed, 27 insertions(+) create mode 100644 app/views/workspace/projects/_shared_organizations.html.erb diff --git a/app/views/workspace/projects/_shared_organizations.html.erb b/app/views/workspace/projects/_shared_organizations.html.erb new file mode 100644 index 00000000..2260af75 --- /dev/null +++ b/app/views/workspace/projects/_shared_organizations.html.erb @@ -0,0 +1,24 @@ +
+
+ <%= t("common.shared_with") %> +
+ <% if @guest_organizations.any? %> + <% @guest_organizations.each do |project_share| %> +
+ <%= project_share.organization.name %> + <%= render ButtonComponent.new( + path: workspace_project_project_share_path(@project, project_share), + method: :delete, + variant: :destructive, + form: { data: { turbo_confirm: "#{t("notice.are_you_sure")} #{t("common.cannot_be_undone")}" } } + ) do %> + <%= t("common.disconnect") %> + <% end %> +
+ <% end %> + <% else %> +
+ <%= t("common.no_shared_organizations") %> +
+ <% end %> +
diff --git a/app/views/workspace/projects/show.html.erb b/app/views/workspace/projects/show.html.erb index 96d41ffa..a06536a4 100644 --- a/app/views/workspace/projects/show.html.erb +++ b/app/views/workspace/projects/show.html.erb @@ -81,6 +81,9 @@ <%= l(@project.created_at, format: "%A, %d %B %Y") %>
+ <% if @guest_organizations.present? %> + <%= render partial: "workspace/projects/shared_organizations" %> + <% end %> <%= render RubyUI::Tabs.new(default: params[:tab] || "tasks") do %> <%= render RubyUI::TabsList.new(class: "border-b h-fit w-full !p-0 rounded-none !justify-start") do %> <%= render RubyUI::TabsTrigger.new(value: "tasks", class!: "h-full py-4 transition duration-300 ease-in-out px-4 data-[state=active]:text-primary-600 data-[state=active]:font-medium border-b-2 border-transparent data-[state=active]:border-primary-600") do %> From af97b2eac299fcdae21746764ad403d5ebb18eaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:33:45 +0100 Subject: [PATCH 26/31] Read-only project show for guest org admin When viewing a shared project (guest org): hide edit/delete buttons and show disconnect action instead, display "Shared by [org]" badge, hide client link and members tab (not relevant for guest org), and add a hook for the rates form partial. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/views/workspace/projects/show.html.erb | 90 ++++++++++++++-------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/app/views/workspace/projects/show.html.erb b/app/views/workspace/projects/show.html.erb index a06536a4..caae4f09 100644 --- a/app/views/workspace/projects/show.html.erb +++ b/app/views/workspace/projects/show.html.erb @@ -8,6 +8,11 @@
<%= @project.name %> <%= render RubyUI::Badge.new(variant: :purple, class: "!rounded-full flex gap-x-1") { t("common.project") } %> + <% if @shared_project %> + <%= render RubyUI::Badge.new(variant: :purple, class: "!rounded-full flex gap-x-1") do %> + <%= t("common.shared_by", org_name: @project.owning_organization.name) %> + <% end %> + <% end %>
<%= @project.description %> @@ -22,11 +27,13 @@ <%= render RubyUI::Breadcrumb::BreadcrumbSeparator.new %> - <%= render RubyUI::Breadcrumb::BreadcrumbItem.new do %> - <%= render RubyUI::Breadcrumb::BreadcrumbLink.new(href: workspace_client_path(@project.client)) { @project.client.name } %> - <% end %> + <% unless @shared_project %> + <%= render RubyUI::Breadcrumb::BreadcrumbItem.new do %> + <%= render RubyUI::Breadcrumb::BreadcrumbLink.new(href: workspace_client_path(@project.client)) { @project.client.name } %> + <% end %> - <%= render RubyUI::Breadcrumb::BreadcrumbSeparator.new %> + <%= render RubyUI::Breadcrumb::BreadcrumbSeparator.new %> + <% end %> <%= render RubyUI::Breadcrumb::BreadcrumbItem.new do %> <%= render RubyUI::Breadcrumb::BreadcrumbPage.new { @project.name } %> @@ -37,18 +44,30 @@
- <%= render ButtonComponent.new(path: edit_workspace_project_path(@project), method: :get, variant: :outline) do %> - - - <% end %> - - <%- has_time_regs = @project.time_regs.any? %> - <%- turbo_body = has_time_regs ? "The project #{@project.name} has time registrations and can not be deleted." : "#{t("notice.deletion_of_he_project")} #{@project.name}, #{t("notice.cannot_be_undone")}" %> - <%= render TooltipComponent.new(note: has_time_regs ? "The Project has recorded time registrations, can not be deleted." : nil) do %> - <%= render ButtonComponent.new(variant: :outline, method: :delete, path: workspace_project_path(@project), disabled: has_time_regs, form: { data: { turbo_confirm: turbo_body } }) do %> - + <% if @shared_project %> + <%= render ButtonComponent.new( + path: workspace_project_project_share_path(@project, @project_share), + method: :delete, + variant: :destructive, + form: { data: { turbo_confirm: "#{t("notice.are_you_sure")} #{t("common.cannot_be_undone")}" } } + ) do %> + <% end %> + <% else %> + <%= render ButtonComponent.new(path: edit_workspace_project_path(@project), method: :get, variant: :outline) do %> + + + <% end %> + + <%- has_time_regs = @project.time_regs.any? %> + <%- turbo_body = has_time_regs ? "The project #{@project.name} has time registrations and can not be deleted." : "#{t("notice.deletion_of_he_project")} #{@project.name}, #{t("notice.cannot_be_undone")}" %> + <%= render TooltipComponent.new(note: has_time_regs ? "The Project has recorded time registrations, can not be deleted." : nil) do %> + <%= render ButtonComponent.new(variant: :outline, method: :delete, path: workspace_project_path(@project), disabled: has_time_regs, form: { data: { turbo_confirm: turbo_body } }) do %> + + + <% end %> + <% end %> <% end %> <%= render ButtonComponent.new(path: reports_path(filter: { project_ids: [ @project.id ] }), method: :get, variant: :outline) do %> @@ -62,10 +81,12 @@
<%= t("common.information")%>
-
- <%= t("common.client")%> - <%= link_to @project.client.name, workspace_client_path(@project.client), class: "font-semibold underline text-primary" %> -
+ <% unless @shared_project %> +
+ <%= t("common.client")%> + <%= link_to @project.client.name, workspace_client_path(@project.client), class: "font-semibold underline text-primary" %> +
+ <% end %>
<%= t("common.rate")%> <%= @currency %> <%= @project.rate_currency %> @@ -84,6 +105,9 @@ <% if @guest_organizations.present? %> <%= render partial: "workspace/projects/shared_organizations" %> <% end %> + <% if @shared_project && current_user.organization_admin? %> + <%= render partial: "workspace/project_shares/rates_form" %> + <% end %> <%= render RubyUI::Tabs.new(default: params[:tab] || "tasks") do %> <%= render RubyUI::TabsList.new(class: "border-b h-fit w-full !p-0 rounded-none !justify-start") do %> <%= render RubyUI::TabsTrigger.new(value: "tasks", class!: "h-full py-4 transition duration-300 ease-in-out px-4 data-[state=active]:text-primary-600 data-[state=active]:font-medium border-b-2 border-transparent data-[state=active]:border-primary-600") do %> @@ -94,13 +118,15 @@
<% end %> - <%= render RubyUI::TabsTrigger.new(value: "members", class!: "h-full py-4 transition duration-300 ease-in-out px-4 data-[state=active]:text-primary-600 data-[state=active]:font-medium border-b-2 border-transparent data-[state=active]:border-primary-600") do %> -
- <%= t("common.members")%> -
- <%= @pagy_members.count %> + <% unless @shared_project %> + <%= render RubyUI::TabsTrigger.new(value: "members", class!: "h-full py-4 transition duration-300 ease-in-out px-4 data-[state=active]:text-primary-600 data-[state=active]:font-medium border-b-2 border-transparent data-[state=active]:border-primary-600") do %> +
+ <%= t("common.members")%> +
+ <%= @pagy_members.count %> +
-
+ <% end %> <% end %> <% end %> <%= render RubyUI::TabsContent.new(value: "tasks") do %> @@ -110,15 +136,17 @@ <%= render PaginationComponent.new(pagy: @pagy_active_assigned_tasks, path: workspace_project_path(@project), params: { tab: "tasks" }) if @pagy_active_assigned_tasks.pages > 1 %> <% end %> - <%= render RubyUI::TabsContent.new(value: "members") do %> -

<%= raw t("common.users_project_access") %>

- <%= content_tag(:div, id: "#{dom_id(@project)}_members", class: "divide-y divide-gray-100") do %> - <%= render partial: "workspace/projects/member", collection: @members %> - <% if @members.empty? %> - <%= render partial: "shared/empty_illustration", locals: { call_to_action: { text: t("common.add_member"), path: edit_workspace_project_path(@project), method: :get } } %> + <% unless @shared_project %> + <%= render RubyUI::TabsContent.new(value: "members") do %> +

<%= raw t("common.users_project_access") %>

+ <%= content_tag(:div, id: "#{dom_id(@project)}_members", class: "divide-y divide-gray-100") do %> + <%= render partial: "workspace/projects/member", collection: @members %> + <% if @members.empty? %> + <%= render partial: "shared/empty_illustration", locals: { call_to_action: { text: t("common.add_member"), path: edit_workspace_project_path(@project), method: :get } } %> + <% end %> <% end %> + <%= render PaginationComponent.new(pagy: @pagy_members, path: workspace_project_path(@project), params: { tab: "members" }) if @pagy_members.pages > 1 %> <% end %> - <%= render PaginationComponent.new(pagy: @pagy_members, path: workspace_project_path(@project), params: { tab: "members" }) if @pagy_members.pages > 1 %> <% end %> <% end %>
From 959c3000b7cd803eac48a005612fab80fe8e8acb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:35:27 +0100 Subject: [PATCH 27/31] Add rate management UI for guest org admin Create rates form partial allowing guest org admins to set project-level and per-task override rates on their project share. The form submits to the update_rates action which now redirects back to the project show page. Eagerly load project_share_task_rates in the show action to avoid N+1 queries. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../workspace/project_shares_controller.rb | 4 +- .../workspace/projects_controller.rb | 4 +- .../project_shares/_rates_form.html.erb | 51 +++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 app/views/workspace/project_shares/_rates_form.html.erb diff --git a/app/controllers/workspace/project_shares_controller.rb b/app/controllers/workspace/project_shares_controller.rb index b22a6f69..4dceb830 100644 --- a/app/controllers/workspace/project_shares_controller.rb +++ b/app/controllers/workspace/project_shares_controller.rb @@ -13,9 +13,9 @@ def index def update_rates authorize! @project_share, to: :update? if @project_share.update(project_share_params) - redirect_to workspace_project_project_shares_path(@project) + redirect_to workspace_project_path(@project), notice: t("notice.project_share_rates_updated") else - redirect_to workspace_project_project_shares_path(@project), alert: t("alert.unable_to_proceed") + redirect_to workspace_project_path(@project), alert: t("alert.unable_to_proceed") end end diff --git a/app/controllers/workspace/projects_controller.rb b/app/controllers/workspace/projects_controller.rb index 6ab19aaf..3069a9da 100644 --- a/app/controllers/workspace/projects_controller.rb +++ b/app/controllers/workspace/projects_controller.rb @@ -31,7 +31,9 @@ def show authorize! @project @shared_project = @project.project_shares.exists?(organization: current_user.current_organization) - @project_share = @project.project_shares.find_by(organization: current_user.current_organization) + @project_share = @project.project_shares + .includes(project_share_task_rates: { assigned_task: :task }) + .find_by(organization: current_user.current_organization) unless @shared_project @guest_organizations = @project.project_shares.includes(:organization) diff --git a/app/views/workspace/project_shares/_rates_form.html.erb b/app/views/workspace/project_shares/_rates_form.html.erb new file mode 100644 index 00000000..cb1905c7 --- /dev/null +++ b/app/views/workspace/project_shares/_rates_form.html.erb @@ -0,0 +1,51 @@ +
+
+ <%= t("common.rates_per_hour") %> +
+ <%= form_with( + url: update_rates_workspace_project_project_share_path(@project, @project_share), + method: :patch, + scope: :project_share, + class: "flex flex-col gap-y-4 py-4" + ) do |form| %> + <%= render RubyUI::FormField.new(class: "flex flex-col") do %> + <%= render RubyUI::FormFieldLabel.new { t("common.project_rate") } %> +
+ <%= @currency %> + <%= form.text_field :rate_currency, + value: @project_share.rate_currency, + class: "border-gray-200 rounded-md w-full", + placeholder: @project.rate_currency %> +
+ <%= render RubyUI::FormFieldHint.new do %> + <%= t("common.optional_project", default: "Leave blank to use the project's default rate") %> + <% end %> + <% end %> + + <% if @project_share.project_share_task_rates.any? %> +
+ <%= t("common.task_rates") %> +
+ <% @project_share.project_share_task_rates.each_with_index do |task_rate, index| %> + <%= form.fields_for :project_share_task_rates_attributes, task_rate, index: index do |task_rate_form| %> + <%= task_rate_form.hidden_field :id, value: task_rate.id %> + <%= render RubyUI::FormField.new(class: "flex flex-col") do %> + <%= render RubyUI::FormFieldLabel.new { task_rate.assigned_task.task.name } %> +
+ <%= @currency %> + <%= task_rate_form.text_field :rate_currency, + value: task_rate.rate_currency, + class: "border-gray-200 rounded-md w-full" %> +
+ <% end %> + <% end %> + <% end %> + <% end %> + +
+ <%= render ButtonComponent.new(variant: :primary) do %> + <%= t("common.update_rates") %> + <% end %> +
+ <% end %> +
From 7a62eedb2b6aba8acb0a50e13b1568b8f756f346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:35:56 +0100 Subject: [PATCH 28/31] Add disconnect action views and project shares index Create index.html.erb for ProjectSharesController replacing the JSON response. Lists shared organizations with disconnect buttons. Update controller to eagerly load organizations and render the template. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../workspace/project_shares_controller.rb | 2 +- .../workspace/project_shares/index.html.erb | 37 +++++++++++++++++++ config/locales/en.yml | 1 - config/locales/nb.yml | 1 - 4 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 app/views/workspace/project_shares/index.html.erb diff --git a/app/controllers/workspace/project_shares_controller.rb b/app/controllers/workspace/project_shares_controller.rb index 4dceb830..e09e8503 100644 --- a/app/controllers/workspace/project_shares_controller.rb +++ b/app/controllers/workspace/project_shares_controller.rb @@ -7,7 +7,7 @@ def index authorize! ProjectShare @project_shares = authorized_scope(ProjectShare, type: :relation) .where(project: @project) - render json: @project_shares + .includes(:organization) end def update_rates diff --git a/app/views/workspace/project_shares/index.html.erb b/app/views/workspace/project_shares/index.html.erb new file mode 100644 index 00000000..517f0953 --- /dev/null +++ b/app/views/workspace/project_shares/index.html.erb @@ -0,0 +1,37 @@ +<% content_for :title, t("common.shared_with") %> + +
+
+
+ <%= t("common.shared_with") %> +
+
+ <%= render ButtonComponent.new(path: workspace_project_path(@project), method: :get, variant: :outline) do %> + + + <% end %> +
+
+ +
+ <% if @project_shares.any? %> + <% @project_shares.each do |project_share| %> +
+ <%= project_share.organization.name %> + <%= render ButtonComponent.new( + path: workspace_project_project_share_path(@project, project_share), + method: :delete, + variant: :destructive, + form: { data: { turbo_confirm: "#{t("notice.are_you_sure")} #{t("common.cannot_be_undone")}" } } + ) do %> + <%= t("common.disconnect") %> + <% end %> +
+ <% end %> + <% else %> +
+ <%= t("common.no_shared_organizations") %> +
+ <% end %> +
+
diff --git a/config/locales/en.yml b/config/locales/en.yml index 746a2b41..9296e020 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -256,7 +256,6 @@ en: shared_project: "Shared project" disconnect: "Disconnect" owned_by: "Owned by %{org_name}" - project_rate: "Project Rate" task_rates: "Task Rates" update_rates: "Update Rates" no_shared_organizations: "This project is not shared with any organizations." diff --git a/config/locales/nb.yml b/config/locales/nb.yml index 4c8e4657..b881fb56 100644 --- a/config/locales/nb.yml +++ b/config/locales/nb.yml @@ -273,7 +273,6 @@ nb: shared_project: "Delt prosjekt" disconnect: "Koble fra" owned_by: "Eid av %{org_name}" - project_rate: "Prosjekttimepris" task_rates: "Oppgavepriser" update_rates: "Oppdater priser" no_shared_organizations: "Dette prosjektet er ikke delt med noen organisasjoner." From 0043c3ed5ba2a52fd6573b962c1438de8a82db18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 16:55:53 +0100 Subject: [PATCH 29/31] fix: allow guest org users to create time_regs on shared projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues fixed: 1. new_modal redirected away if guest org had no own projects — now also checks for shared_projects 2. create? policy denied guest org admins from logging their own time on shared projects — now allows it (still prevents creating on behalf of others) Co-Authored-By: Claude Opus 4.6 (1M context) --- app/controllers/time_regs_controller.rb | 2 +- app/policies/time_reg_policy.rb | 2 +- test/policies/time_reg_policy_test.rb | 18 ++++++++++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/controllers/time_regs_controller.rb b/app/controllers/time_regs_controller.rb index 8eb1fc8d..ce808abf 100644 --- a/app/controllers/time_regs_controller.rb +++ b/app/controllers/time_regs_controller.rb @@ -42,7 +42,7 @@ def new_modal set_assigned_tasks - if current_user.current_organization.projects.empty? + if current_user.current_organization.projects.empty? && current_user.current_organization.shared_projects.empty? flash[:alert] = I18n.t("alert.create_project_before_registering_time") redirect_back fallback_location: time_regs_path end diff --git a/app/policies/time_reg_policy.rb b/app/policies/time_reg_policy.rb index 24277a7f..c7103299 100644 --- a/app/policies/time_reg_policy.rb +++ b/app/policies/time_reg_policy.rb @@ -25,7 +25,7 @@ def create? shared_project = on_shared_project? if user.organization_admin? - same_organization + same_organization || (shared_project && record.user == user) else (same_organization || no_organization || shared_project) && record.user == user end diff --git a/test/policies/time_reg_policy_test.rb b/test/policies/time_reg_policy_test.rb index 982841b4..80ae935e 100644 --- a/test/policies/time_reg_policy_test.rb +++ b/test/policies/time_reg_policy_test.rb @@ -46,7 +46,7 @@ def switch_org_context!(user, organization) assert policy_for(ron, new_time_reg).apply(:create?) end - test "create? org_two admin CANNOT create time_regs on shared project (admin path checks same_organization)" do + test "create? org_two admin CAN create their own time_reg on shared project" do admin = users(:organization_admin) switch_org_context!(admin, @org_two) assert_equal @org_two, admin.current_organization @@ -58,7 +58,21 @@ def switch_org_context!(user, organization) minutes: 60 ) - # Admin path checks same_organization which fails because project belongs to org_one + assert policy_for(admin, new_time_reg).apply(:create?) + end + + test "create? org_two admin CANNOT create time_regs on behalf of another user on shared project" do + admin = users(:organization_admin) + ron = users(:ron) + switch_org_context!(admin, @org_two) + + new_time_reg = TimeReg.new( + user: ron, + assigned_task: assigned_task(:task_1), + date_worked: Date.today, + minutes: 60 + ) + assert_not policy_for(admin, new_time_reg).apply(:create?) end From b11815da55761de127ee05e29cce5c4bbe9cda1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 17:35:34 +0100 Subject: [PATCH 30/31] fix: include shared project time_regs in :own scope The TimeRegPolicy :own scope filtered by organization ID, which excluded time_regs on shared projects (where the project's org differs from the user's current org). Now includes the user's own entries on shared projects via ProjectShare subquery. Also adds Playwright test verifying time_regs are visible on the time_regs index for guest org users. Co-Authored-By: Claude Opus 4.6 (1M context) --- app/policies/time_reg_policy.rb | 11 +- test/playwright/shared_projects.spec.js | 384 ++++++++++++++++++++++++ 2 files changed, 394 insertions(+), 1 deletion(-) create mode 100644 test/playwright/shared_projects.spec.js diff --git a/app/policies/time_reg_policy.rb b/app/policies/time_reg_policy.rb index c7103299..b0b457b6 100644 --- a/app/policies/time_reg_policy.rb +++ b/app/policies/time_reg_policy.rb @@ -49,7 +49,16 @@ def create? scope_for :relation, :own do |relation| organization = user.current_organization - relation.joins(:organization, :user).where(organizations: { id: organization.id }, users: { id: user.id }).distinct + shared_project_ids = ProjectShare.where(organization_id: organization.id).select(:project_id) + + own_ids = relation.joins(:organization, :user) + .where(organizations: { id: organization.id }, users: { id: user.id }) + .select(:id) + shared_ids = relation.joins(assigned_task: :project) + .where(projects: { id: shared_project_ids }, time_regs: { user_id: user.id }) + .select(:id) + + relation.where(id: own_ids).or(relation.where(id: shared_ids)).distinct end private diff --git a/test/playwright/shared_projects.spec.js b/test/playwright/shared_projects.spec.js new file mode 100644 index 00000000..ba68c23b --- /dev/null +++ b/test/playwright/shared_projects.spec.js @@ -0,0 +1,384 @@ +const { test, expect } = require("@playwright/test"); + +// Dev DB state: +// User 1: test@test (admin of "test org") - owns Project 1 "Apollo 11" +// User 2: test123@test (admin of "test org 123") - guest on Project 1 +// Project 1: "Apollo 11" shared with "test org 123" +// Project 2: "My project" owned by "test org 123" + +const USER1 = { email: "test@test", password: "testpass123" }; +const USER2 = { email: "test123@test", password: "testpass123" }; + +async function login(page, user) { + await page.goto("/users/sign_in"); + + // Dismiss cookie consent if present + const declineBtn = page.getByRole("button", { name: "Decline" }); + if (await declineBtn.isVisible({ timeout: 2000 }).catch(() => false)) { + await declineBtn.click(); + } + + await page.fill("#user_email", user.email); + await page.fill("#user_password", user.password); + await page.getByRole("button", { name: "Sign in" }).click(); + await page.waitForURL((url) => !url.pathname.includes("sign_in"), { + timeout: 10000, + }); +} + +// ============================================================ +// OWNING ORG ADMIN (User 1) - Views +// ============================================================ + +test.describe("Owning org admin (User 1 - test org)", () => { + test.beforeEach(async ({ page }) => { + await login(page, USER1); + }); + + test("can see project list with own projects", async ({ page }) => { + await page.goto("/workspace/projects"); + await expect(page.locator("body")).toContainText("Apollo 11"); + }); + + test("can view project show page for owned project", async ({ page }) => { + await page.goto("/workspace/projects/1"); + await expect(page.locator("body")).toContainText("Apollo 11"); + }); + + test("sees guest org name on owned project show page", async ({ page }) => { + await page.goto("/workspace/projects/1"); + await expect(page.locator("body")).toContainText("test org 123"); + }); + + test("can see edit button on owned project", async ({ page }) => { + await page.goto("/workspace/projects/1"); + const editLink = page.locator('a[href="/workspace/projects/1/edit"]'); + await expect(editLink).toBeVisible(); + }); + + test("can access project shares index", async ({ page }) => { + await page.goto("/workspace/projects/1/project_shares"); + expect(page.url()).not.toContain("sign_in"); + await expect(page.locator("body")).toContainText("test org 123"); + }); + + test("can access edit page for owned project", async ({ page }) => { + await page.goto("/workspace/projects/1/edit"); + await expect(page).toHaveURL(/\/workspace\/projects\/1\/edit/); + }); +}); + +// ============================================================ +// GUEST ORG ADMIN (User 2) - Views +// ============================================================ + +test.describe("Guest org admin (User 2 - test org 123)", () => { + test.beforeEach(async ({ page }) => { + await login(page, USER2); + }); + + test("can see own projects in project list", async ({ page }) => { + await page.goto("/workspace/projects"); + await expect(page.locator("body")).toContainText("My project"); + }); + + test("project list page contains shared indicator", async ({ page }) => { + await page.goto("/workspace/projects"); + // The page should show something related to sharing + const pageText = await page.locator("body").textContent(); + // Apollo 11 might appear under a different grouping or with a shared badge + const hasContent = + pageText.includes("Apollo") || + pageText.includes("Shared") || + pageText.includes("test org"); + expect(hasContent).toBeTruthy(); + }); + + test("can view shared project show page (read-only)", async ({ page }) => { + await page.goto("/workspace/projects/1"); + await expect(page.locator("body")).toContainText("Apollo 11"); + expect(page.url()).toContain("/workspace/projects/1"); + }); + + test("does NOT see edit button on shared project", async ({ page }) => { + await page.goto("/workspace/projects/1"); + const editLink = page.locator('a[href="/workspace/projects/1/edit"]'); + await expect(editLink).toHaveCount(0); + }); + + test("cannot access edit page for shared project (redirected)", async ({ + page, + }) => { + await page.goto("/workspace/projects/1/edit"); + expect(page.url()).not.toContain("/edit"); + }); + + test("can view own project show page", async ({ page }) => { + await page.goto("/workspace/projects/2"); + await expect(page.locator("body")).toContainText("My project"); + }); + + test("can see edit button on own project", async ({ page }) => { + await page.goto("/workspace/projects/2"); + const editLink = page.locator('a[href="/workspace/projects/2/edit"]').first(); + await expect(editLink).toBeVisible(); + }); +}); + +// ============================================================ +// TIME REGISTRATION - Guest org user +// ============================================================ + +test.describe("Time registration on shared project (User 2)", () => { + test.beforeEach(async ({ page }) => { + await login(page, USER2); + }); + + test("time_regs index loads without redirect", async ({ page }) => { + await page.goto("/time_regs"); + await expect(page).toHaveURL(/\/time_regs/); + expect(page.url()).not.toContain("sign_in"); + }); + + test("time_regs index shows entries on shared project", async ({ page }) => { + await page.goto("/time_regs"); + // User 2 has time_regs on the shared project "Apollo 11" + // The page should show time entries (not be empty) + await expect(page.locator("body")).toContainText("Apollo 11"); + }); + + test("update_tasks_select returns tasks for shared project", async ({ + page, + }) => { + await page.goto("/time_regs"); + + const response = await page.evaluate(async () => { + const res = await fetch( + "/time_regs/update_tasks_select?project_id=1", + { headers: { Accept: "text/html" } } + ); + return { status: res.status, body: await res.text() }; + }); + + expect(response.status).toBe(200); + expect(response.body).toContain("Welding"); + }); + + test("can create a time_reg on shared project", async ({ page }) => { + await page.goto("/time_regs"); + + const csrfToken = await page.evaluate(() => { + const meta = document.querySelector('meta[name="csrf-token"]'); + return meta ? meta.getAttribute("content") : null; + }); + expect(csrfToken).toBeTruthy(); + + const response = await page.evaluate( + async ({ token }) => { + const today = new Date().toISOString().split("T")[0]; + const formData = new FormData(); + formData.append("time_reg[assigned_task_id]", "1"); + formData.append("time_reg[date_worked]", today); + formData.append("time_reg[minutes]", "45"); + formData.append("time_reg[notes]", "Playwright test entry"); + formData.append("time_reg[project_id]", "1"); + + const res = await fetch("/time_regs", { + method: "POST", + headers: { + "X-CSRF-Token": token, + Accept: + "text/vnd.turbo-stream.html, text/html, application/xhtml+xml", + }, + body: formData, + redirect: "follow", + }); + return { status: res.status, url: res.url, ok: res.ok }; + }, + { token: csrfToken } + ); + + expect(response.ok).toBeTruthy(); + expect(response.url).toContain("/time_regs"); + }); +}); + +// ============================================================ +// RATE MANAGEMENT - Guest org admin +// ============================================================ + +test.describe("Rate management (User 2 - guest org admin)", () => { + test.beforeEach(async ({ page }) => { + await login(page, USER2); + }); + + test("sees rate-related content on shared project page", async ({ + page, + }) => { + await page.goto("/workspace/projects/1"); + const bodyText = await page.locator("body").textContent(); + const hasRateUI = + bodyText.includes("Rate") || + bodyText.includes("rate") || + bodyText.includes("€"); + expect(hasRateUI).toBeTruthy(); + }); + + test("can submit rate update via fetch", async ({ page }) => { + // Navigate first to get a session cookie + await page.goto("/workspace/projects/1"); + + const csrfToken = await page.evaluate(() => { + const meta = document.querySelector('meta[name="csrf-token"]'); + return meta ? meta.getAttribute("content") : null; + }); + expect(csrfToken).toBeTruthy(); + + const response = await page.evaluate( + async ({ token }) => { + const params = new URLSearchParams(); + params.append("project_share[rate_currency]", "7.50"); + params.append("_method", "patch"); + params.append("authenticity_token", token); + + const res = await fetch( + "/workspace/projects/1/project_shares/1/update_rates", + { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + Accept: "text/html, application/xhtml+xml", + }, + body: params.toString(), + redirect: "follow", + } + ); + return { status: res.status, url: res.url, ok: res.ok }; + }, + { token: csrfToken } + ); + + expect(response.ok).toBeTruthy(); + }); +}); + +// ============================================================ +// REPORTS - Guest org admin +// ============================================================ + +test.describe("Reports for guest org (User 2)", () => { + test.beforeEach(async ({ page }) => { + await login(page, USER2); + }); + + test("reports page loads successfully", async ({ page }) => { + await page.goto("/reports"); + await expect(page).toHaveURL(/\/reports/); + expect(page.url()).not.toContain("sign_in"); + }); +}); + +// ============================================================ +// AUTHORIZATION - Verify restrictions +// ============================================================ + +test.describe("Authorization restrictions", () => { + test("guest org admin cannot delete shared project", async ({ page }) => { + await login(page, USER2); + + const csrfToken = await page.evaluate(() => { + const meta = document.querySelector('meta[name="csrf-token"]'); + return meta ? meta.getAttribute("content") : null; + }); + + const response = await page.evaluate( + async ({ token }) => { + const res = await fetch("/workspace/projects/1", { + method: "DELETE", + headers: { + "X-CSRF-Token": token, + Accept: "text/html", + }, + redirect: "follow", + }); + return { status: res.status, url: res.url }; + }, + { token: csrfToken } + ); + + expect(response.url).not.toMatch(/\/workspace\/projects\/1$/); + }); + + test("guest org admin cannot update shared project", async ({ page }) => { + await login(page, USER2); + + const csrfToken = await page.evaluate(() => { + const meta = document.querySelector('meta[name="csrf-token"]'); + return meta ? meta.getAttribute("content") : null; + }); + + const response = await page.evaluate( + async ({ token }) => { + const formData = new FormData(); + formData.append("project[name]", "Hacked Name"); + + const res = await fetch("/workspace/projects/1", { + method: "PATCH", + headers: { + "X-CSRF-Token": token, + Accept: "text/html", + }, + body: formData, + redirect: "follow", + }); + return { status: res.status, url: res.url }; + }, + { token: csrfToken } + ); + + expect(response.url).not.toContain("/workspace/projects/1/edit"); + }); + + test("owning org admin can still access edit page", async ({ page }) => { + await login(page, USER1); + await page.goto("/workspace/projects/1/edit"); + await expect(page).toHaveURL(/\/workspace\/projects\/1\/edit/); + }); +}); + +// ============================================================ +// EXPORT - Downloads trigger correctly +// ============================================================ + +test.describe("Export functionality", () => { + test("owning org admin can export project time_regs", async ({ page }) => { + await login(page, USER1); + await page.goto("/time_regs"); + + // Use fetch to test the endpoint returns CSV data + const response = await page.evaluate(async () => { + const res = await fetch("/time_regs/export?project_id=1", { + headers: { Accept: "text/csv, text/html" }, + }); + return { status: res.status, contentType: res.headers.get("content-type") }; + }); + + expect(response.status).toBe(200); + }); + + test("guest org admin can export shared project time_regs", async ({ + page, + }) => { + await login(page, USER2); + await page.goto("/time_regs"); + + const response = await page.evaluate(async () => { + const res = await fetch("/time_regs/export?project_id=1", { + headers: { Accept: "text/csv, text/html" }, + }); + return { status: res.status, contentType: res.headers.get("content-type") }; + }); + + expect(response.status).toBe(200); + }); +}); From 9cea3380007bc1dfe9673ad548b08acf6234a53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20H=C3=B8glund?= Date: Tue, 17 Mar 2026 18:09:09 +0100 Subject: [PATCH 31/31] refactor: address code review findings 1. Fix missing locale key: use notice.project_share_rates_updated 2. Extract ProjectShare#rate_for_task and Project#project_share_for to reduce N+1 queries and improve message passing 3. Use model methods (shared_with?, project_share_for, owning_organization, has_accessible_projects?) instead of reaching into associations from controllers, policies, and views 4. Show guest org's rate on shared project info section 5. Rename update_rates to standard RESTful update action 6. Extract switch_org_context! to test_helper.rb (was duplicated in 9 files) Co-Authored-By: Claude Opus 4.6 (1M context) --- app/controllers/time_regs_controller.rb | 2 +- .../project_share_task_rates_controller.rb | 4 ++-- .../workspace/project_shares_controller.rb | 8 ++++---- .../workspace/projects_controller.rb | 11 ++++++---- app/models/organization.rb | 4 ++++ app/models/project.rb | 9 +++++++++ app/models/project_share.rb | 6 ++++++ app/models/time_reg.rb | 10 ++-------- app/policies/workspace/project_policy.rb | 2 +- .../project_shares/_rates_form.html.erb | 2 +- app/views/workspace/projects/show.html.erb | 6 +++++- config/routes.rb | 5 +---- test/controllers/time_regs_controller_test.rb | 12 ----------- ...roject_share_task_rates_controller_test.rb | 6 ------ .../project_shares_controller_test.rb | 20 +++++++------------ .../workspace/projects_controller_test.rb | 6 ------ test/playwright/shared_projects.spec.js | 2 +- test/policies/task_policy_test.rb | 6 ------ test/policies/time_reg_policy_test.rb | 6 ------ .../policies/workspace/project_policy_test.rb | 6 ------ .../workspace/project_share_policy_test.rb | 6 ------ .../project_share_task_rate_policy_test.rb | 6 ------ test/test_helper.rb | 6 +++++- 23 files changed, 56 insertions(+), 95 deletions(-) diff --git a/app/controllers/time_regs_controller.rb b/app/controllers/time_regs_controller.rb index ce808abf..546c580e 100644 --- a/app/controllers/time_regs_controller.rb +++ b/app/controllers/time_regs_controller.rb @@ -42,7 +42,7 @@ def new_modal set_assigned_tasks - if current_user.current_organization.projects.empty? && current_user.current_organization.shared_projects.empty? + unless current_user.current_organization.has_accessible_projects? flash[:alert] = I18n.t("alert.create_project_before_registering_time") redirect_back fallback_location: time_regs_path end diff --git a/app/controllers/workspace/project_share_task_rates_controller.rb b/app/controllers/workspace/project_share_task_rates_controller.rb index 62323230..b1b1e834 100644 --- a/app/controllers/workspace/project_share_task_rates_controller.rb +++ b/app/controllers/workspace/project_share_task_rates_controller.rb @@ -9,7 +9,7 @@ def create authorize! @task_rate if @task_rate.save - redirect_to workspace_project_path(@project), notice: t("notice.rates_updated") + redirect_to workspace_project_path(@project), notice: t("notice.project_share_rates_updated") else redirect_to workspace_project_path(@project), alert: t("alert.unable_to_proceed") end @@ -19,7 +19,7 @@ def update authorize! @task_rate if @task_rate.update(task_rate_params) - redirect_to workspace_project_path(@project), notice: t("notice.rates_updated") + redirect_to workspace_project_path(@project), notice: t("notice.project_share_rates_updated") else redirect_to workspace_project_path(@project), alert: t("alert.unable_to_proceed") end diff --git a/app/controllers/workspace/project_shares_controller.rb b/app/controllers/workspace/project_shares_controller.rb index e09e8503..47ddd4c0 100644 --- a/app/controllers/workspace/project_shares_controller.rb +++ b/app/controllers/workspace/project_shares_controller.rb @@ -1,7 +1,7 @@ module Workspace class ProjectSharesController < WorkspaceController before_action :set_project - before_action :set_project_share, only: %i[update_rates destroy] + before_action :set_project_share, only: %i[update destroy] def index authorize! ProjectShare @@ -10,8 +10,8 @@ def index .includes(:organization) end - def update_rates - authorize! @project_share, to: :update? + def update + authorize! @project_share if @project_share.update(project_share_params) redirect_to workspace_project_path(@project), notice: t("notice.project_share_rates_updated") else @@ -48,7 +48,7 @@ def project_share_params end def owner_org? - @project.organization == current_user.current_organization + @project.owning_organization == current_user.current_organization end end end diff --git a/app/controllers/workspace/projects_controller.rb b/app/controllers/workspace/projects_controller.rb index 3069a9da..8c9818b1 100644 --- a/app/controllers/workspace/projects_controller.rb +++ b/app/controllers/workspace/projects_controller.rb @@ -30,10 +30,13 @@ def create def show authorize! @project - @shared_project = @project.project_shares.exists?(organization: current_user.current_organization) - @project_share = @project.project_shares - .includes(project_share_task_rates: { assigned_task: :task }) - .find_by(organization: current_user.current_organization) + @shared_project = @project.shared_with?(current_user.current_organization) + @project_share = @project.project_share_for(current_user.current_organization) + + if @shared_project && @project_share + @project_share = ProjectShare.includes(project_share_task_rates: { assigned_task: :task }) + .find(@project_share.id) + end unless @shared_project @guest_organizations = @project.project_shares.includes(:organization) diff --git a/app/models/organization.rb b/app/models/organization.rb index d9f7f864..7dd5688a 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -12,6 +12,10 @@ class Organization < ApplicationRecord validates :name, presence: true, uniqueness: true validate :currency_exists + def has_accessible_projects? + projects.any? || shared_projects.any? + end + def currency_exists errors.add(:currency, "is not a valid currency") unless Stemplin.config.currencies.keys.include?(self.currency&.to_sym) end diff --git a/app/models/project.rb b/app/models/project.rb index 922344b8..5c2f13b1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -37,10 +37,19 @@ def shared_with?(organization) end end + def project_share_for(organization) + if project_shares.loaded? + project_shares.find { |ps| ps.organization_id == organization.id } + else + project_shares.find_by(organization: organization) + end + end + def owning_organization organization end + def must_have_at_least_one_active_assigned_task errors.add(:tasks, :blank) if assigned_tasks.to_a.reject { |assigned_task| assigned_task.marked_for_destruction? || assigned_task.is_archived }.empty? end diff --git a/app/models/project_share.rb b/app/models/project_share.rb index bb26c4d5..56f80912 100644 --- a/app/models/project_share.rb +++ b/app/models/project_share.rb @@ -10,6 +10,12 @@ class ProjectShare < ApplicationRecord validates :organization_id, uniqueness: { scope: :project_id } validate :organization_is_not_project_owner + def rate_for_task(assigned_task) + task_rate = project_share_task_rates.find_by(assigned_task: assigned_task) + rate = task_rate&.rate || 0 + rate.positive? ? rate : self.rate + end + private def organization_is_not_project_owner diff --git a/app/models/time_reg.rb b/app/models/time_reg.rb index 89cc078f..b02165d5 100644 --- a/app/models/time_reg.rb +++ b/app/models/time_reg.rb @@ -78,14 +78,8 @@ def used_rate end def used_rate_for(organization) - project_share = project.project_shares.find_by(organization: organization) - if project_share - task_rate = project_share.project_share_task_rates.find_by(assigned_task: assigned_task) - rate = task_rate&.rate || 0 - rate.positive? ? rate : project_share.rate - else - used_rate - end + project_share = project.project_share_for(organization) + project_share ? project_share.rate_for_task(assigned_task) : used_rate end def total_hours diff --git a/app/policies/workspace/project_policy.rb b/app/policies/workspace/project_policy.rb index ffdf525e..e4a34281 100644 --- a/app/policies/workspace/project_policy.rb +++ b/app/policies/workspace/project_policy.rb @@ -12,7 +12,7 @@ class ProjectPolicy < WorkspacePolicy def show? return true if user.organization_admin? && record.organization == user.current_organization - return true if user.organization_admin? && record.project_shares.exists?(organization: user.current_organization) + return true if user.organization_admin? && record.shared_with?(user.current_organization) false end diff --git a/app/views/workspace/project_shares/_rates_form.html.erb b/app/views/workspace/project_shares/_rates_form.html.erb index cb1905c7..1965996a 100644 --- a/app/views/workspace/project_shares/_rates_form.html.erb +++ b/app/views/workspace/project_shares/_rates_form.html.erb @@ -3,7 +3,7 @@ <%= t("common.rates_per_hour") %> <%= form_with( - url: update_rates_workspace_project_project_share_path(@project, @project_share), + url: workspace_project_project_share_path(@project, @project_share), method: :patch, scope: :project_share, class: "flex flex-col gap-y-4 py-4" diff --git a/app/views/workspace/projects/show.html.erb b/app/views/workspace/projects/show.html.erb index caae4f09..c5285516 100644 --- a/app/views/workspace/projects/show.html.erb +++ b/app/views/workspace/projects/show.html.erb @@ -89,7 +89,11 @@ <% end %>
<%= t("common.rate")%> - <%= @currency %> <%= @project.rate_currency %> + <% if @shared_project && @project_share %> + <%= @currency %> <%= @project_share.rate_currency %> + <% else %> + <%= @currency %> <%= @project.rate_currency %> + <% end %>
<%= t("common.billing_status")%> diff --git a/config/routes.rb b/config/routes.rb index 15300234..b9144f3e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -60,10 +60,7 @@ namespace :workspace do resources :projects do post :import_modal, on: :collection - resources :project_shares, only: [ :index, :destroy ] do - member do - patch :update_rates - end + resources :project_shares, only: [ :index, :update, :destroy ] do resources :project_share_task_rates, only: [ :create, :update ] end end diff --git a/test/controllers/time_regs_controller_test.rb b/test/controllers/time_regs_controller_test.rb index 28da00dd..e920b494 100644 --- a/test/controllers/time_regs_controller_test.rb +++ b/test/controllers/time_regs_controller_test.rb @@ -206,12 +206,6 @@ class SharedProjectMember < TimeRegsControllerTest assert_includes project_names, @shared_project.name end - private - - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end end class SharedProjectAdmin < TimeRegsControllerTest @@ -253,11 +247,5 @@ class SharedProjectAdmin < TimeRegsControllerTest end end - private - - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end end end diff --git a/test/controllers/workspace/project_share_task_rates_controller_test.rb b/test/controllers/workspace/project_share_task_rates_controller_test.rb index 84ec28fd..9b9f9b08 100644 --- a/test/controllers/workspace/project_share_task_rates_controller_test.rb +++ b/test/controllers/workspace/project_share_task_rates_controller_test.rb @@ -15,12 +15,6 @@ class ProjectShareTaskRatesControllerTest < ActionController::TestCase @org_two = organizations(:organization_two) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- create --- test "create as org_two admin creates a task rate on the project share" do diff --git a/test/controllers/workspace/project_shares_controller_test.rb b/test/controllers/workspace/project_shares_controller_test.rb index 209a4638..c386eef1 100644 --- a/test/controllers/workspace/project_shares_controller_test.rb +++ b/test/controllers/workspace/project_shares_controller_test.rb @@ -14,12 +14,6 @@ class ProjectSharesControllerTest < ActionController::TestCase @org_two = organizations(:organization_two) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- index --- test "index as org_one admin lists guest orgs for the project" do @@ -39,14 +33,14 @@ def switch_org_context!(user, organization) end end - # --- update_rates --- + # --- update --- - test "update_rates as org_two admin updates project_share rate and task rates" do + test "update as org_two admin updates project_share rate and task rates" do switch_org_context!(@organization_admin, @org_two) sign_in @organization_admin task_rate = project_share_task_rates(:task_rate_one) - patch :update_rates, params: { + patch :update, params: { project_id: @project.id, id: @project_share.id, project_share: { @@ -62,8 +56,8 @@ def switch_org_context!(user, organization) assert_equal 300, task_rate.reload.rate end - test "update_rates as org_one admin denied" do - patch :update_rates, params: { + test "update as org_one admin denied" do + patch :update, params: { project_id: @project.id, id: @project_share.id, project_share: { rate_currency: "5.00" } @@ -73,10 +67,10 @@ def switch_org_context!(user, organization) assert_redirected_to root_path end - test "update_rates calls authorize!" do + test "update calls authorize!" do switch_org_context!(@organization_admin, @org_two) assert_authorized_to(:update?, @project_share, with: Workspace::ProjectSharePolicy) do - patch :update_rates, params: { + patch :update, params: { project_id: @project.id, id: @project_share.id, project_share: { rate_currency: "5.00" } diff --git a/test/controllers/workspace/projects_controller_test.rb b/test/controllers/workspace/projects_controller_test.rb index ee607f3c..a998fc7b 100644 --- a/test/controllers/workspace/projects_controller_test.rb +++ b/test/controllers/workspace/projects_controller_test.rb @@ -93,12 +93,6 @@ class ProjectsControllerTest < ActionController::TestCase # --- Shared project tests (guest org admin) --- - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - test "org_two admin can access show for a shared project" do shared_project = projects(:project_1) switch_org_context!(@organization_admin, organizations(:organization_two)) diff --git a/test/playwright/shared_projects.spec.js b/test/playwright/shared_projects.spec.js index ba68c23b..1910bccf 100644 --- a/test/playwright/shared_projects.spec.js +++ b/test/playwright/shared_projects.spec.js @@ -242,7 +242,7 @@ test.describe("Rate management (User 2 - guest org admin)", () => { params.append("authenticity_token", token); const res = await fetch( - "/workspace/projects/1/project_shares/1/update_rates", + "/workspace/projects/1/project_shares/1", { method: "POST", headers: { diff --git a/test/policies/task_policy_test.rb b/test/policies/task_policy_test.rb index 0a1e25f0..e4a32bbf 100644 --- a/test/policies/task_policy_test.rb +++ b/test/policies/task_policy_test.rb @@ -24,12 +24,6 @@ def scoped_tasks(user) policy.apply_scope(Task.all, type: :relation) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- Scope: org_two admin sees tasks assigned to shared projects --- test "scope: org_two admin sees tasks assigned to shared projects (org_one's tasks)" do diff --git a/test/policies/time_reg_policy_test.rb b/test/policies/time_reg_policy_test.rb index 80ae935e..dc9afd7e 100644 --- a/test/policies/time_reg_policy_test.rb +++ b/test/policies/time_reg_policy_test.rb @@ -22,12 +22,6 @@ def scoped_time_regs(user) policy.apply_scope(TimeReg.all, type: :relation) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- create? --- test "create? org_two member can create their own time_reg on shared project" do diff --git a/test/policies/workspace/project_policy_test.rb b/test/policies/workspace/project_policy_test.rb index 8a1dc962..f78728e0 100644 --- a/test/policies/workspace/project_policy_test.rb +++ b/test/policies/workspace/project_policy_test.rb @@ -21,12 +21,6 @@ def scoped_projects(user) policy.apply_scope(Project.all, type: :relation) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- Scope tests --- test "scope: org_one admin sees all org_one projects" do diff --git a/test/policies/workspace/project_share_policy_test.rb b/test/policies/workspace/project_share_policy_test.rb index 20b99a93..e50ffa02 100644 --- a/test/policies/workspace/project_share_policy_test.rb +++ b/test/policies/workspace/project_share_policy_test.rb @@ -14,12 +14,6 @@ def policy_for(user, record = @project_share) Workspace::ProjectSharePolicy.new(record, user: user) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- show? --- test "show? allowed for org_one admin (project owner)" do diff --git a/test/policies/workspace/project_share_task_rate_policy_test.rb b/test/policies/workspace/project_share_task_rate_policy_test.rb index ed1403b4..4c030076 100644 --- a/test/policies/workspace/project_share_task_rate_policy_test.rb +++ b/test/policies/workspace/project_share_task_rate_policy_test.rb @@ -15,12 +15,6 @@ def policy_for(user, record = @task_rate) Workspace::ProjectShareTaskRatePolicy.new(record, user: user) end - # Helper to switch a user's active context to a given organization - def switch_org_context!(user, organization) - user.access_infos.update_all(active: false) - user.access_infos.find_by(organization: organization).update!(active: true) - end - # --- create? --- test "create? allowed for org_two admin (guest org)" do diff --git a/test/test_helper.rb b/test/test_helper.rb index 01732f20..21f69fac 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -31,7 +31,11 @@ class ActiveSupport::TestCase # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. fixtures :all - # Add more helper methods to be used by all tests here... + # Switch a user's active organization context for cross-org testing + def switch_org_context!(user, organization) + user.access_infos.update_all(active: false) + user.access_infos.find_by(organization: organization).update!(active: true) + end end