use cluster title or id.titleize #4614
Conversation
| label: 'Cluster', | ||
| options: clusters.map(&:id) | ||
| options: clusters.map do |c| | ||
| [c.metadata.title || c.id.to_s.titleize, c.id] |
There was a problem hiding this comment.
We likely need safe traversal here as c.metadata may itself be nil.
|
Thinking about this again we should bury this in ood_core. I've reopened this ticket: OSC/ood_core#899 with a comment. |
| label: 'Cluster', | ||
| options: clusters.map do |c| | ||
| [c.metadata.title || c.id.to_s.titleize, c.id] | ||
| [c&.metadata&.title || c.id.to_s.titleize, c.id] |
There was a problem hiding this comment.
I feel like since we're putting this in ood_core it's just [c.title, c.id] right? Though this'll be on hold until that API exists.
There was a problem hiding this comment.
yes it will be, so we could either put this on hold until then or merge it now and know that we will be writing over this section before 4.1 is finished. I would also mention that we don't do safe calls on metadata.title anywhere else in the codebase, but I am happy to play it safe here until we get a chance to consider it carefully there.
There was a problem hiding this comment.
we could either put this on hold until then or merge it now and know that we will be writing over this section before 4.1 is finished
Yea I'd say to hold it.
I would also mention that we don't do safe calls on metadata.title anywhere else
Yea that's what happens when there are reviews or a 2nd pair of eyes!
Fixes #1648. Mimics the approach taken in
ondemand/apps/dashboard/app/apps/ood_app.rb
Line 131 in ee4d3e7
for accessing cluster title but defaulting to id.titleize. While this hurts the style, we will be able to get our terseness back when we implement #4613