Skip to content

Fixes #39266 - Expose partition table snippet flag in API - #11276

Merged
lzap merged 1 commit into
theforeman:developfrom
jakduch:feature/ptable-snippet-api
Oct 12, 2026
Merged

lzap merged 1 commit into
theforeman:developfrom
jakduch:feature/ptable-snippet-api

Conversation

@jakduch

@jakduch jakduch commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

What

  • expose the snippet flag in partition table show, create, and update responses
  • verify that creating and updating a partition table persists the flag and returns it through the API

Why

The API already accepts the snippet parameter, but omits it from partition table responses. API clients therefore cannot read or confirm the stored state.

Testing

  • ruby -c test/controllers/api/v2/ptables_controller_test.rb
  • git diff --check
  • The focused Rails test requires the Foreman bundle, which is not installed in this clean worktree; it is left to CI.

AI assistance disclosure:

  • Assisted by Codex 5.6 Sol High.

@jakduch
jakduch force-pushed the feature/ptable-snippet-api branch from 2b2cf45 to ca94207 Compare September 22, 2026 23:17
@jakduch

jakduch commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Updated the API update test to clear os_family when converting the fixture to a snippet, since snippets require it to be blank. New commit: ca942075a.

@jakduch

jakduch commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

The CI failures are unrelated to the partition-table API change. Both JavaScript jobs hit the same time-dependent DateTimePicker test. The remaining failures are separate existing flakes: HostgroupJSTest timed out after all retries and Katello hit an unhandled VCR request in AlternateContentSourceRefreshTest; the Test suite failures are only the aggregate result. I am preparing a separate fix for the DateTimePicker test.

@jakduch

jakduch commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

The JavaScript matrix is failing on the shared DateTimePicker midnight-boundary test, unrelated to this PR. I fixed that deterministic failure in #11280. The Ruby/integration failures are separate upstream test failures and are being tracked independently.

@jakduch

jakduch commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

The matrix failures are the unrelated DateTimePicker future-only midnight-boundary failure fixed by #11280 (Redmine #39792). The Ruby 3.3 integration failure is the shared HostgroupJSTest timeout fixed by #11291 (Redmine #39803). The remaining Katello failure is an unrelated VCR cassette mismatch in AlternateContentSourceRefreshTest; this PR only exposes the partition-table snippet field in the API.

@jakduch

jakduch commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

The independent Katello VCR failure is now resolved upstream by Katello/katello#11867, merged as a4a5ed2. That change fixed the new Pulp binding request-body defaults that broke body_json cassette matching, and its Ruby 3.0/3.3 Katello suites passed. The remaining Foreman-side failures are covered by #11280 and #11291.

@jakduch
jakduch force-pushed the feature/ptable-snippet-api branch from ca94207 to 46827e1 Compare September 26, 2026 11:36

@lzap lzap left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks.

Can you also update the index action so the flag is exposed there?

@ofedoren

Copy link
Copy Markdown
Member

I guess it won't hurt to add this to the hammer part as well: https://github.com/theforeman/hammer-cli-foreman/blob/master/lib/hammer_cli_foreman/partition_table.rb

@jakduch
jakduch force-pushed the feature/ptable-snippet-api branch from 46827e1 to 54bee42 Compare September 30, 2026 09:47
@jakduch

jakduch commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, snippet is now exposed by the shared partition table view, so it is included in both index and show responses. I added index coverage and rebased onto current develop. I will handle the Hammer exposure in a separate hammer-cli-foreman PR.

@jakduch

jakduch commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

The Hammer follow-up is ready in theforeman/hammer-cli-foreman#663. It exposes snippet in both partition table list and info output.

@jakduch
jakduch force-pushed the feature/ptable-snippet-api branch from 54bee42 to 84bff4b Compare September 30, 2026 11:52
@jakduch

jakduch commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

The functional failure was in the new assertion: the index response wraps records in results, so the test was checking an entry from the metadata hash instead of a partition table. Fixed that and squashed the branch back to one commit.

Expose the snippet flag from partition table API index and show responses and cover the serializer output with a functional test.\n\nAssisted-By: Codex 6 Astra
@jakduch
jakduch force-pushed the feature/ptable-snippet-api branch from 5a107a0 to 0fae5f4 Compare September 30, 2026 16:14
@jakduch

jakduch commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Updated: the snippet flag is now exposed from the shared main response used by the index action, with an index assertion in the controller test. The full CI is green.

@lzap lzap left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lzap
lzap merged commit b9eed79 into theforeman:develop Oct 12, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants