Handle missing group param in version routes and align branch files with RuboCop

[Copilot]

/node/version/view and /node/version/diffs could raise when group= was omitted because downstream code expected a string-like value. This change set hardens group handling for those routes and cleans up style offenses in the two branch-modified files.

• Route input normalization (lib/oxidized/web/webapp.rb)

  • Normalize params[:group] to '' at request parsing boundaries.
  • Convert empty group to nil only when calling backend diff/version APIs that expect nil for ungrouped nodes.
  • Keep view-layer behavior consistent by ensuring @info[:group] is never nil.

• Regression coverage (spec/web/node/version_spec.rb)

  • Add/retain coverage for /node/version/view and /node/version/diffs requests where group is absent.
  • Assert ungrouped diff calls pass nil to get_diff.

• RuboCop compliance in changed files

  • Apply the remaining auto-correctable offense in spec/web/node/version_spec.rb (hash key alignment), leaving logic unchanged.

@info = {
  node: node,
  group: params[:group] || '',
  oid: params[:oid]
}

the_data = nodes.get_version node, (@info[:group].empty? ? nil : @info[:group]), @info[:oid]

Original prompt

Please run bundle exec rubocop on the fix/nil-group-param-nomethoderror branch and fix any offenses in the files changed by this branch:

• lib/oxidized/web/webapp.rb
• spec/web/node/version_spec.rb

The branch fixes a NoMethodError caused by params[:group] being nil when the group= query param is absent from /node/version/view and /node/version/diffs requests.

Steps:

1. Run bundle exec rubocop lib/oxidized/web/webapp.rb spec/web/node/version_spec.rb
2. If there are any offenses that cannot be auto-corrected, fix them manually
3. Run bundle exec rubocop --auto-correct lib/oxidized/web/webapp.rb spec/web/node/version_spec.rb to apply auto-corrections
4. Commit any changes needed to make rubocop pass cleanly
5. Do NOT modify any files outside of lib/oxidized/web/webapp.rb and spec/web/node/version_spec.rb unless rubocop requires it

If rubocop already passes with no offenses, just confirm that and make no changes.

The following is the prior conversation context from the user's chat exploration (may be truncated):

Assistant: ```dockerfile
RUN
# ── Patch oxidized-web: nil group param causes NoMethodError ──────────────
# ytti#441
# https://github.com/makaiver/oxidized-web/tree/fix/nil-group-param-nomethoderror
# Safe to remove once a fixed oxidized-web gem version is installed.
WEBAPP=$(ls "${GEM_HOME}"/gems/oxidized-web-*/lib/oxidized/web/webapp.rb 2>/dev/null || true);
if [ -z "${WEBAPP}" ]; then
echo "oxidized-web webapp.rb not found - skipping nil-group patches.";
else
if grep -q "group: params[:group]," "${WEBAPP}" 2>/dev/null; then
echo "Applying nil-group NoMethodError patch to oxidized-web...";
sed -i "s/group: params[:group],/group: (params[:group] || ''),/g" "${WEBAPP}";
echo "Patch 1/2 applied.";
else
echo "Patch 1/2 not needed (already fixed upstream or pattern changed) - skipping.";
fi;
if grep -q "nodes.get_diff node, @info[:group]" "${WEBAPP}" 2>/dev/null; then
echo "Applying get_diff nil-group patch to oxidized-web...";
sed -i "s/nodes.get_diff node, @info[:group]/nodes.get_diff node, (@info[:group].empty? ? nil : @info[:group])/g" "${WEBAPP}";
echo "Patch 2/2 applied.";
else
echo "Patch 2/2 not needed (already fixed upstream or pattern changed) - skipping.";
fi;
fi

User: can we confirm sed patcjis correct

Assistant: Confirmed. Here's the full verification against the actual file:

**Patch 1 — detection string:**

grep: "group: params[:group],"

Original gem lines 163 & 184: `group: params[:group],` ✅ matches

After sed, becomes: `group: (params[:group] || ''),` ✅ correct

---

**Patch 2 — detection string:**

grep: "nodes.get_diff node, @info[:group]"

Original gem lines 204 & 206: `nodes.get_diff node, @info[:group], @info[:oid], ...` ✅ matches

After sed, becomes: `nodes.get_diff node, (@info[:group].empty? ? nil : @info[:group]), @info[:oid], ...` ✅ correct

---

**One thing to note** — the sed output differs slightly from the branch fix:

| | Line 204/206 result |
|---|---|
| **Branch** | `nodes.get_diff node, group, ...` (reuses local var) |
| **Sed patch** | `nodes.get_diff node, (@info[:group].empty? ? nil : @info[:group]), ...` (inline ternary) |

Both are **functionally identical** — `group` local var on line 189–190 does exactly the same `'' → nil` conversion as the inline ternary. The sed approach is actually more self-contained since it doesn't rely on the local `group` variable existing, which is safer as a blind patch.

**Both patches are correct. ✅**

User: @ytti/oxidized-web/pull/441 is checklist fine?

Assistant: Here's the full checklist assessment for [ytti/oxidized-web#441](https://github.com/ytti/oxidized-web/pull/441):

---

### Pre-Request Checklist

| Item | Status | Notes |
|---|---|---|
| ✅ Passes rubocop | ☑️ Claimed | No CI failure detected |
| ✅ Tests added/adapted | ☑️ Claimed | PR body says minitest cases added in `spec/web/node/version_spec.rb` |
| ✅ CHANGELOG.md updated | ☑️ Claimed | PR body says changes appended |

All three boxes are ticked by the author.

---

### Code diff — what actually landed

The PR diff includes **all 3 fixes** we identified (including the `get_diff` one we added):

| Line | Change |
|---|---|
| 163 | `group: params[:group] || ''` ✅ |
| 169 | `nodes.get_version node, @info[:group].empty? ? nil : @info[:group], ...` ✅ |
| 184 | `group: params[:group] || ''` ✅ |
| 204 | `nodes.get_diff node, group, ...` ✅ |
| 206 | `nodes.get_diff node, group, ...` ✅ |

---

### Merge readiness

| | |
|---|---|
| **CI** | ✅ No failing checks |
| **Reviews** | ⏳ None yet — no approvals, no changes requested |
| **Mergeable** | ✅ No conflicts |
| **Draft** | No |

Only thing missing is a maintainer review/approval from `ytti`. The code and checklist are solid. 🟡 Waiting on review...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>