Skip to content
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

Towards consistency for displaying test entities. Stop hiding test entities on some dashboards #23724

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jun 8, 2022

Overview

There is no consistency as to whether test entities are shown on (eg. contact tabs). Some do and some don't. This makes test entities rather less useful because you can't see the results.
Eg. in the case of an event signup you don't see the participant record in the contact events tab if it is a test. But you do see the test contributions (but not in the user dashboard..).

This PR changes those that were hidden based on the "force" parameter so that test entities are not filtered out and are displayed. This provides consistency and standardisation.

In most cases the UI will already show ( test ) or make it clear in some way that the entity is a test.

Before

Very inconsistent display of test entities.

After

Consistent display of test entities.

Technical Details

This has always annoyed me, using the "_force" param (which is meant for triggering an immediate search) to decide whether to show test entities in some situations. And makes for some awkward conversations when training people on CiviCRM - eg. "You'll see that it's created a test contribution but you won't see the event registration because that's a test and Civi hides those. So why can I see the contribution? Because civi doesn't hide those? But it does in the dashboard? Yes, but only for contriubutions..."

Comments

@civibot
Copy link

civibot bot commented Jun 8, 2022

(Standard links)

@civibot civibot bot added the master label Jun 8, 2022
@mattwire
Copy link
Contributor Author

mattwire commented Jun 8, 2022

[CRM_Contact_Page_View_UserDashBoardTest.testDashboardContentEmptyContact](https://test.civicrm.org/job/CiviCRM-Core-PR/49573/testReport/junit/(root)/CRM_Contact_Page_View_UserDashBoardTest/testDashboardContentEmptyContact/)
[CRM_Contact_Page_View_UserDashBoardTest.testDashboardContentContributionsWithInvoicingEnabled](https://test.civicrm.org/job/CiviCRM-Core-PR/49573/testReport/junit/(root)/CRM_Contact_Page_View_UserDashBoardTest/testDashboardContentContributionsWithInvoicingEnabled/)
[CRM_Contact_Page_View_UserDashBoardTest.testDashboardContentContributions](https://test.civicrm.org/job/CiviCRM-Core-PR/49573/testReport/junit/(root)/CRM_Contact_Page_View_UserDashBoardTest/testDashboardContentContributions/)
[CRM_Contact_Page_View_UserDashBoardTest.testDashboardPartialPayments](https://test.civicrm.org/job/CiviCRM-Core-PR/49573/testReport/junit/(root)/CRM_Contact_Page_View_UserDashBoardTest/testDashboardPartialPayments/)

@demeritcowboy
Copy link
Contributor

Maybe it's all the forest-fire smoke I've been breathing but when this PR first went up I didn't think too much about it since it seemed like more of a developer thing. But I'm now a medium-hard +1 on it. (Read that as you will.)

jenkins retest this please

My thinking is if real users don't use test mode anyway (which anecdotally is true), then this doesn't really change anything on most installs.

@colemanw
Copy link
Member

I'll add my +1 as well. Make mine over-easy 🍳

@demeritcowboy
Copy link
Contributor

demeritcowboy commented Jun 30, 2023

I think you've coined a new phrase. An "over-easy opinion".

jenkins retest this please (date rollover issue, probably delayed trying to process what the comments are talking about).

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Jun 30, 2023
@mattwire
Copy link
Contributor Author

mattwire commented Jul 3, 2023

@demeritcowboy thanks for picking this up - looks like tests are all passing now :-)

@demeritcowboy demeritcowboy merged commit bd4c3e5 into civicrm:master Jul 3, 2023
@mattwire mattwire deleted the eventtest branch July 4, 2023 10:22
@mattwire
Copy link
Contributor Author

mattwire commented Jul 4, 2023

Thanks @demeritcowboy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants