-
Notifications
You must be signed in to change notification settings - Fork 4
Allow arbitrary project linkage #281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
miles-smith
wants to merge
18
commits into
develop
Choose a base branch
from
project-relationship-graphs
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
97660bb
Create join model for relating projects to one another
d4d515c
Add ProjectRelationship associations to Project
112e435
Add ProjectEdge model
411b6ec
Wire up related projects association(s)
4f8b655
Add commentary on relationship associations
10c2431
Populate project edges for existing ODR projects
cda5e68
Add transitive closure scope to ProjectEdge
fcfacbc
Allow transitive closure to be restricted to unique edges
9dc5306
Refactor existing related project views
6ade893
Lazy attempt to order related projects by ODR reference
8ceeafe
Include path prefix in project layout
8f4ee82
Add bare minimum UI functionality for linking projects
503f574
AJAX-ify project relationship creation/deletion
fa0ca6c
Boyscouting/performance
1dcefa8
Merge branch 'develop' into project-relationship-graphs
miles-smith 15495f9
Remove redundant clone_of field on application form
c364f88
Boyscouting - performance/style
64dbd18
Merge branch 'develop' into project-relationship-graphs
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| # RESTfully manages creating/deleting `ProjectRelationship`s | ||
| class ProjectRelationshipsController < ApplicationController | ||
| load_and_authorize_resource :project | ||
| load_and_authorize_resource :team, through: :project, singleton: true | ||
| load_and_authorize_resource through: :project, only: %i[create destroy] | ||
|
|
||
| def index | ||
| authorize!(:read, ProjectRelationship) | ||
|
|
||
| @projects = @team.projects. | ||
| accessible_by(current_ability). | ||
| where.not(id: @project). | ||
| search(search_params). | ||
| order_by_reference. | ||
| includes(:project_type, :owner, :current_state). | ||
| paginate(page: params[:page], per_page: 10) | ||
| end | ||
|
|
||
| def create | ||
| @other_project = @team.projects.find_by(id: resource_params[:right_project_id]) | ||
|
|
||
| @project_relationship.assign_attributes( | ||
| left_project: @project, | ||
| right_project: @other_project | ||
| ) | ||
|
|
||
| if @project_relationship.save | ||
| respond_to do |format| | ||
| format.html { redirect_to after_action_path, notice: t('.success') } | ||
| format.js | ||
| end | ||
| else | ||
| respond_to do |format| | ||
| format.html { redirect_to after_action_path, alert: t('.failure') } | ||
| format.js | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def destroy | ||
| @other_project = @project_relationship.projects.detect { |project| project != @project } | ||
|
|
||
| if @project_relationship.destroy | ||
| respond_to do |format| | ||
| format.html { redirect_to after_action_path, notice: t('.success') } | ||
| format.js | ||
| end | ||
| else | ||
| respond_to do |format| | ||
| format.html { redirect_to after_action_path, alert: t('.failure') } | ||
| format.js | ||
| end | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def resource_params | ||
| params.require(:project_relationship).permit(:right_project_id) | ||
| end | ||
|
|
||
| def search_params | ||
| params.fetch(:search, {}).permit( | ||
| :name, | ||
| :application_log, | ||
| project_type_id: [], | ||
| owner: %i[ | ||
| first_name | ||
| last_name | ||
| ], | ||
| current_project_state: { | ||
| state_id: [] | ||
| } | ||
| ) | ||
| end | ||
| helper_method :search_params | ||
|
|
||
| def after_action_path | ||
| project_project_relationships_path(@project, search: search_params) | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # Endpoint for fetching the related projects for a given resource. | ||
| class RelatedProjectsController < ApplicationController | ||
| load_and_authorize_resource :project | ||
|
|
||
| def index | ||
| edges = @project.project_edges.transitive_closure | ||
| projects = Project.from(Project.accessible_by(current_ability), :projects). | ||
| where(id: edges.select(:related_project_id)). | ||
| order_by_reference. | ||
| includes(:project_type, :current_state) | ||
|
|
||
| locals = { | ||
| projects: projects | ||
| } | ||
|
|
||
| respond_to do |format| | ||
| format.html { render partial: 'projects', locals: locals, content_type: :html } | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # Helper class built upon a view over the `project_relationships` table, which pivots those records | ||
| # into something a little easier to query because: | ||
| # - managing a join model is easy. | ||
| # - querying both sides of a join table because a resource may exist on either FK is hard. | ||
| # - querying an adjaceny list is easy. | ||
| # - managing adjaceny lists for inverse pairs of records via ActiveRecord callbacks is like | ||
| # sticking your head inside an aligator's mouth; it's all good until it inevitably bites. | ||
| class ProjectEdge < ApplicationRecord | ||
| belongs_to :project_relationship | ||
| belongs_to :project | ||
| belongs_to :related_project, class_name: 'Project' | ||
|
|
||
| class << self | ||
| # Returns an expanded set of `ProjectEdge`s for `project`, containing edges for both directly | ||
| # related projects and those that are indirectly related through one or more intermediates. | ||
| # Related projects may be returned multiple times if it can be reached via many different | ||
| # paths. Passing the `distinct` option with a truthy value will limit the returned set to only | ||
| # include unique related projects (selected by the shortest path). | ||
| # | ||
| # NOTE: This can be a costly query and as such may be a performance problem area. Will probably | ||
| # want to see/capture some stats from usage in the wild... | ||
| def transitive_closure_for(project, **options) | ||
| from(<<~SQL.squish) | ||
| ( | ||
| #{transitive_closure_cte(project)} | ||
| #{transitive_closure_query(distinct: options[:distinct])} | ||
| ) #{quoted_table_name} | ||
| SQL | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def transitive_closure_cte(project) | ||
| sanitize_sql([<<~SQL.squish, project.id]) | ||
| WITH RECURSIVE transitive_closure(project_id, related_project_id, distance, path) AS ( | ||
| SELECT project_id | ||
| , related_project_id | ||
| , 1 AS distance | ||
| , ARRAY[project_id, related_project_id] AS path | ||
| FROM project_edges | ||
| WHERE project_id = ? | ||
|
|
||
| UNION ALL | ||
|
|
||
| SELECT t.project_id | ||
| , e.related_project_id | ||
| , t.distance + 1 | ||
| , t.path || e.related_project_id AS path | ||
| FROM project_edges e | ||
| JOIN transitive_closure t on e.project_id = t.related_project_id | ||
| WHERE NOT t.path && ARRAY[e.related_project_id] | ||
| ) | ||
| SQL | ||
| end | ||
|
|
||
| def transitive_closure_query(distinct: false) | ||
| if distinct | ||
| <<~SQL.squish | ||
| SELECT DISTINCT ON (related_project_id) * | ||
| FROM transitive_closure | ||
| ORDER BY related_project_id, distance | ||
| SQL | ||
| else | ||
| <<~SQL.squish | ||
| SELECT * | ||
| FROM transitive_closure | ||
| SQL | ||
| end | ||
| end | ||
| end | ||
|
|
||
| # This model is backed by a database view. | ||
| def readonly? | ||
| true | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Join model representing some relationship between two `Project`s. | ||
| class ProjectRelationship < ApplicationRecord | ||
| with_options class_name: 'Project' do | ||
| belongs_to :left_project | ||
| belongs_to :right_project | ||
| end | ||
|
|
||
| has_many :project_edges, inverse_of: :project_relationship # rubocop:disable Rails/HasManyOrHasOneDependent | ||
| has_many :projects, through: :project_edges, source: :related_project | ||
|
|
||
| validate :no_inverse_pairs | ||
| validate :no_self_referential_relationships | ||
|
|
||
| private | ||
|
|
||
| def no_inverse_pairs | ||
| return unless left_project_id && right_project_id | ||
|
|
||
| return unless | ||
| self.class.where( | ||
| left_project_id: left_project_id, | ||
| right_project_id: right_project_id | ||
| ). | ||
| or( | ||
| self.class.where( | ||
| left_project_id: right_project_id, | ||
| right_project_id: left_project_id | ||
| ) | ||
| ). | ||
| exists? | ||
|
|
||
| errors.add(:base, :taken) | ||
miles-smith marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| end | ||
|
|
||
| def no_self_referential_relationships | ||
| return unless left_project_id && right_project_id | ||
| return if left_project_id != right_project_id | ||
|
|
||
| errors.add(:base, :self_referential) | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <% | ||
| relationship = subject.relationship_to(project) | ||
| %> | ||
|
|
||
| <%= content_tag(:tr, class: dom_class(project), id: dom_id(project)) do %> | ||
| <td><%= project_type_label(project) %></td> | ||
| <td><%= project.application_log %></td> | ||
| <%= tag.td(project.name, class: :'no-overflow', title: project.name) %> | ||
| <%= tag.td(project.owner_full_name, class: :'no-overflow', title: project.owner_full_name) %> | ||
| <td><%= project_status_label(project) %></td> | ||
| <td> | ||
| <% if relationship %> | ||
| <%= delete_link(relationship, path: polymorphic_path([subject, relationship], search: search_params), title: 'Remove', text: true, icon: :'minus-sign', class: 'btn btn-danger btn-xs btn-block', remote: true, data: { disable: true }) %> | ||
| <% else %> | ||
| <%= form_with(url: project_project_relationships_path(subject, search: search_params), scope: :project_relationship, local: false) do |form| %> | ||
| <%= form.hidden_field(:right_project_id, value: project.id) %> | ||
| <button type="submit" class="btn btn-primary btn-xs btn-block" data-disable="true"> | ||
| <%= bootstrap_icon_tag(:'plus-sign') %> | ||
| Add | ||
| </button> | ||
| <% end %> | ||
| <% end %> | ||
| </td> | ||
| <% end %> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <% | ||
| html = render 'project', project: @other_project, subject: @project | ||
| %> | ||
|
|
||
| $('tr#<%= j(dom_id(@other_project)) %>').replaceWith('<%= j(html) %>'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <% | ||
| html = render 'project', project: @other_project, subject: @project | ||
| %> | ||
|
|
||
| $('tr#<%= j(dom_id(@other_project)) %>').replaceWith('<%= j(html) %>'); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More of a question than anything - would where.not(id: @project.id) be more efficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot, and you're absolutely right.
It's still one DB hit, but the difference is in loading one record into memory rather than two. There are times when relying on the preloaded/cached association would be beneficial but this isn't one of those instances :D
This code was a remnant from when associations weren't as fleshed out as they are now and
project_relationships.projectswas just an enumerable wrapper around the twobelongs_toassociations.I'll push a commit with your suggested change 👍