Skip to content

Improve Spree::Preferences constants and source code documentation - #6696

Merged
jarednorman merged 3 commits into
solidusio:mainfrom
benjaminwil:preference-documentation-improvements
Oct 23, 2026
Merged

jarednorman merged 3 commits into
solidusio:mainfrom
benjaminwil:preference-documentation-improvements

Conversation

@benjaminwil

Copy link
Copy Markdown
Contributor

Summary

While I have been building out payment method interfaces for the new solidus_admin gem, I had reason to dig into how preferences work. I needed to test that forms were working correctly against every supported preference type.

I noticed that some of the documentation was unclear or could be improved.

I also noticed some constants related to Spree::Preferences functionality could be renamed to act as better documentation for these features, too.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@github-actions github-actions Bot added the changelog:solidus_core Changes to the solidus_core gem label Oct 16, 2026
@benjaminwil
benjaminwil force-pushed the preference-documentation-improvements branch from 3bda527 to 5f00236 Compare October 16, 2026 02:24
@benjaminwil benjaminwil changed the title Improve Spree::Prefrences constants and source code documentation Improve Spree::Preferences constants and source code documentation Oct 16, 2026
@benjaminwil
benjaminwil force-pushed the preference-documentation-improvements branch from 5f00236 to 6b57fb1 Compare October 16, 2026 03:59
@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.46%. Comparing base (76f4a18) to head (e9a73d6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6696   +/-   ##
=======================================
  Coverage   92.46%   92.46%           
=======================================
  Files        1058     1058           
  Lines       21495    21495           
=======================================
  Hits        19875    19875           
  Misses       1620     1620           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@benjaminwil
benjaminwil marked this pull request as ready for review October 16, 2026 04:36
@benjaminwil
benjaminwil requested a review from a team as a code owner October 16, 2026 04:36
# @return [Array]
def allowed_admin_form_preference_types = Spree::Preferences::FORM_FIELD_TYPES
def allowed_admin_form_preference_types
Spree::Preferences::FORM_FIELD_TYPES.dup

@benjaminwil benjaminwil Oct 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After coming to this fresh today, I realized that this breaks current functionality in payment method extensions.

So I intend to drop this change.

I'll instead create a separate pull request to deprecate this method and make allowed form preference types a proper configuration point on Spree::AppConfiguration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To clarify: by "this change" I just mean the commit where I add the #dup call.

@benjaminwil
benjaminwil force-pushed the preference-documentation-improvements branch from 6b57fb1 to b2229c3 Compare October 19, 2026 22:17
@jarednorman

Copy link
Copy Markdown
Member

Real failures on CI?

@benjaminwil

benjaminwil commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

@jarednorman Yeah, real. Merge conflict after #6695 was merged. Will fix this today.

I found the old `DEFAULT_ADMIN_FORM_PREFERENCE_TYPES` constant to be a
bit buried given how critical the information it contains is. (It's used
across Solidus libraries for calculators, promotions, and payment
methods.)

It serves as better documentation as a part of the main
`Spree::Preferences` class where the rest of the documentation for
preferences live.

This also clarify exactly what preferences are explicitly supported by
Solidus. Before this, you had to dig through a case statement to
see the full list.
While I was referring to the preferences documentation I found a lot of
detail that muddied the purpose and usage instructions. This commit
rewrites the docs and removes some superfluous and repeated docs.
Hopefully this makes the documentation more maintainable over time.

I also wanted to add explicit reference to preferences within the
payment method class documentation. Hopefully this helps orient
developers working on payment method integrations so that they
understand better:

- which preference types are built-in.
- where the seams are for making admin-editable payment method records

In subsequent commits, I will add additional documentation for where
admin view components will be expected for applications using
`solidus_admin`. There is only one preference to the legacy
`preference_fields/` partials directory now, so there is now only one
place to add references to the new admin's fields directory.
@benjaminwil
benjaminwil force-pushed the preference-documentation-improvements branch from b2229c3 to e9a73d6 Compare October 20, 2026 19:33
@benjaminwil

benjaminwil commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

I amended a test assertion introduced when #6695 was merged. It was using one of the old constants that I removed in this pull request.

No other changes.

@jarednorman
jarednorman merged commit 6f5b2b9 into solidusio:main Oct 23, 2026
43 checks passed
Noah-Silvera pushed a commit to SuperGoodSoft/solidus that referenced this pull request Nov 2, 2026
…tation-improvements

Improve `Spree::Preferences` constants and source code documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:solidus_core Changes to the solidus_core gem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants