Add "Manage Premiums" to AdminUI, toward dev/core#3912 .

[twomice]

Overview

Add "Manage Premiums" to AdminUI

Before

After

Technical Details

None.

Comments

None. Should be ready to review now.

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[UshaMakoa]

Nice Thank you

But (sorry I'm testing my first review on your PR ;-) ) :

• General standards
  • Explain (r-explain)
    • PASS : The goal/problem/solution have been adequately explained in the PR.
  • User impact (r-user)
    • PASS: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
  • Documentation (r-doc)
    • PASS: The changes do not require documentation.
  • Run it (r-run)
    • ISSUE:

1. I'm not sure the Action menu is useful?
2. For the "add product" button in your SK/SearchDisplay (/civicrm/admin/search#/edit/26), in the toolbar,
   @aydun has added the add, edit, delete to the CRM/Contribute/xml/Menu/Contribute.xml in the PR 28544 to add 'civicrm/admin/contribute/managePremiums/edit' and reference add, edit, browse, delete actions. can you update this PR to use the new paths?
3. The links in the Menu (at the end of each row) doesn't function well : Preview, Edit, Delete (but Disable function). For Delete and Edit the bulk SK actions should function now. For Preview : 'civicrm/admin/contribute/managePremiums/edit?action=preview&reset=1'
4. You can delete the help text (blue one) in the SK/SearchDisplay (field Description) and leave the help text (green one) in the FB rich content but it would be better to ts() this text please. here is an example :

<div>{{:: ts('The status rules provided by default may be sufficient for your organization. However, you can easily change the status names and\/or adjust the rules by clicking the Edit links below. Or you can add a new status and rule.') }}</div>
</div>

• Developer standards
  • Technical impact (r-tech)
    • UNREVIEWED
  • Code quality (r-code)
    • UNREVIEWED
  • Maintainability (r-maint)
    • UNREVIEWED
  • Test results (r-test)
    • UNREVIEWED

[twomice]

Thanks for the review @UshaMakoa . I've removed "WIP" ("work in progress") from the title, and I think now it's actually ready. With help from @aydun I was able to address the items you mentioned, and a little more.

i suggest this is ready for review now.

[twomice]

Thanks for the help @aydun !

[colemanw]

@twomice thanks for this PR it looks very good.
Just one thing: our new convention is to declare afforms using php files instead of json so that strings can be translated. So afsearchPremiumProducts.aff.json should be renamed like #28499 - also the title should be wrapped in E::ts() and all the null values (and the modified_date) can be deleted for concision.

[twomice]

@colemanw I've made those changes now; hopefully this is good, but please let me know if more is needed. Thanks!

[aydun]

The paths in the XML and DAO are different: they should have the /edit part as in the DAO. However, these are now updated in master except for the preview link. Can you try rebasing on master and only adding the preview link?

[twomice]

@aydun I've made that change now (if I understand you correctly), and functionality seems to be correct. Thanks for the feedback! Any further requirements, please let me know.

[colemanw]

This path doesn't seem to match the one in the generated DAO file.

[colemanw]

We need to match this to whatever permission was being checked previously for this screen.
I find the menu xml files a bit too magical in the way they assign permissions to paths, but I think the way it works is that the standard url inherits its permissions from the next path in the hierarchy that has them declared, and that would be civicrm/admin. And that one checks administer CiviCRM system + administer CiviCRM data + access CiviCRM. That seems a bit silly. Maybe we should set this to

Suggested change

	'access CiviCRM',
	'access CiviContribute',
	'administer CiviCRM data',

[colemanw]

@twomice I think we're very close on this one. if you could just check those last 2 comments, I think that's all that's left to do before merging.

[eileenmcnaughton]

@colemanw if you want to merge this & make those changes in a follow up I'll merge the follow up

[colemanw]

Ok I fixed the path. Lets merge this now & try to fix the Afform permissions during the RC period.

[twomice]

Thanks; sorry I couldn't get to this quicker.