roadmap
roadmap copied to clipboard
An issue with GuidanceController admin_index() method
In app/models/guidance_group.rb we have the method
def self.create_org_default(org) GuidanceGroup.create!( name: org.abbreviation? ? org.abbreviation : org.name, org: org, optional_subset: false ) end
In app/controllers/guidances_controller.rb the above method is called by method
def ensure_default_group(org) return unless org.managed? return if org.guidance_groups.where(optional_subset: false).present?
GuidanceGroup.create_org_default(org)
end
which is called by the controller method
GET /org/admin/guidance/:id/admin_index def admin_index authorize Guidance @guidances = Guidance.by_org(current_user.org) .includes(:guidance_group, :themes).page(1) ensure_default_group(current_user.org) @guidance_groups = GuidanceGroup.by_org(current_user.org).page(1) end
In DCC we had the following issue comment https://github.com/DigitalCurationCentre/DMPonline-Service/issues/625#issuecomment-953013169
If the GuidanceGroup has variable optional_subset: true
gg = GuidanceGroup.where(name: "MIUN") GuidanceGroup Load (0.9ms) SELECT "guidance_groups".* FROM "guidance_groups" WHERE "guidance_groups"."name" = $1 LIMIT $2 [["name", "MIUN"], ["LIMIT", 11]] => #<ActiveRecord::Relation [#<GuidanceGroup id: 480245895, name: "MIUN", org_id: 1071580137, created_at: "2021-04-26 12:29:36", updated_at: "2021-04-26 12:29:59", optional_subset: true, published: false>]>
This causes the method ensure_default_group in app/controllers/guidances_controller.rb* fails because of error ActiveRecord::RecordInvalid (Validation failed: Name must be unique) we cannot access the Guidance pages by an Admin.
irb(main):002:0> GuidanceGroup.create!(name:mu.abbreviation, org: mu, optional_subset: false) (CALL THAT FAILS IN CODE) (0.1ms) SAVEPOINT active_record_1 GuidanceGroup Exists (0.4ms) SELECT 1 AS one FROM "guidance_groups" WHERE "guidance_groups"."name" = $1 AND "guidance_groups"."org_id" = $2 LIMIT $3 [["name", "MIUN"], ["org_id", 1071580137], ["LIMIT", 1]] (0.2ms) ROLLBACK TO SAVEPOINT active_record_1 Traceback (most recent call last): 1: from (irb):2 ActiveRecord::RecordInvalid (Validation failed: Name must be unique)
This suggest an issue for the method ensure_default_group in app/controllers/guidances_controller.rb*.
@briri & @raycarrick-ed I am not clear how the GuidanceGroup GuidanceGroup.where(name: "MIUN") got into state optional_subset: true => #<ActiveRecord::Relation [#<GuidanceGroup id: 480245895, name: "MIUN", org_id: 1071580137, created_at: "2021-04-26 12:29:36", updated_at: "2021-04-26 12:29:59", optional_subset: true, published: false>]>
The optional_subset
flag is a way to identify that the GuidanceGroup is a child of another GuidanceGroup. It is a poor implementation though and has always been problematic because its not a true association.
There must always be a GuidanceGroup with optional_subset = false
to act as the "parent" so that it can be displayed as:
Some University
|
L___> Some school at the University
I'm guessing that in these scenarios there is not GuidanceGroup with an optional_subset of false. The user can change this value when editing the record. Another issue could be if there were multiple GuidanceGroups with the optional_subset of false ... which one is the correct parent?
For the short term, you could try updating the logic on that ensure
method to allow for either optional_subset value. Not sure how it will properly render on the view though. Or, force that value to false in the case where there is only one GuidanceGroup.
Longer term, it would be a good idea to setup a true self referential relationship on that table. It might also make sense at this point to optionally allow those child guidance groups to be associated with departments.
Cheers @briri for comment. Might have time to look at next week.
maybe that filter, https://github.com/DMPRoadmap/roadmap/blob/master/app/controllers/guidances_controller.rb#L143, should not filter on optional_subset: false
, as that incorrectly returns false, and assumes that the default guidance group doesn't exist yet, and tries to make one (which fails on validation). And yes, you could force that default group to optional_subset: false
if it was set incorrectly..