New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change Price paths #26929
Change Price paths #26929
Conversation
Thank you for contributing to CiviCRM! ❤️ We will need to test and review the PR. 👷 Introduction for new contributors
Quick links for reviewers |
xml/schema/Price/PriceFieldValue.xml
Outdated
<add>civicrm/admin/price/field/option/edit?reset=1&action=add&fid=[PriceFieldValue_PriceField_price_field_id_01.id]&sid=[PriceFieldValue_PriceField_price_field_id_01.price_set_id]</add> | ||
<view>civicrm/admin/price/field/option/edit?reset=1&action=view&oid=[id]&fid=[price_field_id]&sid=[PriceFieldValue_PriceField_price_field_id_01.price_set_id]</view> | ||
<update>civicrm/admin/price/field/option/edit?reset=1&action=update&oid=[id]&fid=[price_field_id]&sid=[PriceFieldValue_PriceField_price_field_id_01.price_set_id]</update> | ||
<delete>civicrm/admin/price/field/option/edit?reset=1&action=delete&oid=[id]&fid=[price_field_id]&sid=[PriceFieldValue_PriceField_price_field_id_01.price_set_id]</delete> | ||
<browse>civicrm/admin/price/field/option</browse> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aydun it looks like this path is referencing an explicit join. Explicit joins don't have a stable name that can be referenced like this (since the name of the join is anything you want it to be when you call ->addJoin()
in the API). Is it possible to do this with an implicit join, like [price_field_id.price_set_id]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That does work for the view, update and delete links but not for the add
one. Any chance you could tweak how things are processed for the add button to make that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ADD button processes tokens differently than links, because it doesn't have any row data to use (it's not in a row) the only data it has for tokens is the incoming filters. I know this works for a few other pages, e.g Custom Fields has an incoming filter by custom_group_id
and that is passed through to the ADD button.
So if you switch your page filter to use implicit-join-style syntax then they should get passed through that way to the button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm - I can't get it to work for that one. Maybe there's something else not quite right in the metadata? I've changed it as you suggest. How about we merge this then come back to the add button in another PR with the new pages?
b52f32e
to
45e24b7
Compare
Ok @aydun I'm happy to merge this but it looks like it'll need an upgrader for the navigation items. |
- fix unreleased regression relating to adding new price set fields and values - add Upgrader Switch html type from Select to EntityRef for Price Fields and Values
- fix unreleased regression relating to adding new price set fields and values - add Upgrader Switch html type from Select to EntityRef for Price Fields and Values
- fix unreleased regression relating to adding new price set fields and values - add Upgrader Switch html type from Select to EntityRef for Price Fields and Values
- fix unreleased regression relating to adding new price set fields and values - add Upgrader Switch html type from Select to EntityRef for Price Fields and Values
Overview
Change paths for Price Sets, Price Fields and Price Field Values
Before
After
There should be no user-visible changes, but this allows the original URL's to be overridden by AdminUI.
Technical Details
This is similar to other path changes that have been done to accommodate AdminUI.
Comments