Skip to content

service_body shortcode updates - #262

Merged
otrok7 merged 1 commit into
bmlt-enabled:mainfrom
nadams5755:main
Oct 27, 2026
Merged

otrok7 merged 1 commit into
bmlt-enabled:mainfrom
nadams5755:main

Conversation

@nadams5755

@nadams5755 nadams5755 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

support new service body shortcodes to match formatting from BMLT while not impacting existing use cases:

  • update service_body_X menu item to suffix with (UPPER CASE)
  • add service_body_X_name to match the name as written in BMLT
  • add service_body_x_description from BMLT
  • add service_body_x_helpline from BMLT
  • add helper functions to lookup service body fields from BMLT
  • update test fixtures
  • add tests for new functions
  • add tests for new shortcodes

it's been a long time since i wrote php. i worked with claude to make these changes. tested with docker on linux:

docker compose up -d
docker run --rm -u "$(id -u):$(id -g)" -v "$PWD":/app -w /app composer:2 install --ignore-platform-req=ext-gd
docker compose exec -e XDEBUG_MODE=off -w /var/www/html/wp-content/plugins/bread wordpress vendor/bin/phpunit --display-warnings --display-deprecations --display-notices

same warnings/notices as HEAD~1, so nothing introduced here.

@otrok7 otrok7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great idea.

Probably want the helpline in there, too.

I think I would be a bit more bold and replace get_areas with the new get_service_body more generally. That's a much deeper change, though. I don't know if you want that to be in this PR...

@nadams5755

Copy link
Copy Markdown
Contributor Author

the contribution guide doesn't say but: if i add the helpline, should i add a commit to the branch/pr or should i amend the existing commit?

as for replacing get_areas: i'd probably do two it as two separate PRs. one PR to update all the tests to fix the warnings and notices. another PR to do the consolidation work.

@otrok7

otrok7 commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

To be clear: nothing wrong with the PR as is, and the functionality it adds is useful. I'm more thinking about how to share the work than how to package it in commits/branches/PRs. I'm thrilled to have another dev even loosely familiar with this code. I don't want to pressure you to do more than you want, but I don't want to discourage you from doing more by running myself with these extras. So my question is more, should I just approve this PR and add the other stuff myself or do you want to continue?

As far as git goes, I usually work in branches for the next release, adding a version bump before merging.

support new service body shortcodes to match formatting from BMLT while not impacting existing use cases:
* update service_body_X menu item to suffix with (UPPER CASE)
* add service_body_X_name to match the name as written in BMLT
* add service_body_x_description from BMLT
* add service_body_x_helpline from BMLT
* add helper functions to lookup service body fields from BMLT
* update test fixtures
* add tests for new functions
* add tests for new shortcodes
@nadams5755

Copy link
Copy Markdown
Contributor Author

i added the helpline bits, good suggestion. tests passed. i amended the commit. as for further contributions, i'll start by trying to clean up the tests in another PR.

@otrok7
otrok7 merged commit eeb0e6b into bmlt-enabled:main Oct 27, 2026
1 check 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.

2 participants