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

Fix activity_date_time_relative filter when `preserve_activity_tab_fi… #20602

Merged
merged 1 commit into from
Jul 1, 2021

Conversation

sluc23
Copy link
Contributor

@sluc23 sluc23 commented Jun 14, 2021

…lter` setting is on

Overview

When this setting is on Administer / Display Preferences / Customize Data and Screens / Preserve activity filters as a user preference the Activity Date filter is not properly saved.

Before

Replicated in dmaster:

activity_filter

After

Activity Date filter is properly saved.

@civibot
Copy link

civibot bot commented Jun 14, 2021

(Standard links)

@civibot civibot bot added the master label Jun 14, 2021
@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS
      • There's a pre-existing weirdness where viewing the activities dashlet won't use your settings but at the same time will clobber your settings. It's not from this PR.
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS with suggestion
      • This is pre-existing but at line 441 when it sets the setting, it's possible for $activityFilter to have never been set, e.g. if you have no filters set on the activity tab. This generates an error every time someone first visits an activity tab after turning on the setting. Suggest putting $activityFilter = []; before the loop around line 421.
    • [r-maint] ?
      • It would be nice to have a test. It seems like you could get your setup variable by logging var_export($_REQUEST, true) from a live run, and then in the test set $_REQUEST to that and then call the function and then check the settings values.
    • [r-test] PASS

@sluc23 sluc23 force-pushed the dev#PreserveActivityTabFilter branch from 9e7b09c to 3af2689 Compare July 1, 2021 17:01
@sluc23
Copy link
Contributor Author

sluc23 commented Jul 1, 2021

@demeritcowboy :

  • [r-code]: fixed
  • [r-maint] : I'm struggling to do the test, because I dont' find a way where I can set up the setting preserve_activity_tab_filter, then setup some activity filters, and then check that the activity filters are still there.. 🤔

@colemanw
Copy link
Member

colemanw commented Jul 1, 2021

I think this is ok without a test.

@demeritcowboy
Copy link
Contributor

Ok thanks for trying. I was thinking maybe it would be an easy one.

@demeritcowboy demeritcowboy merged commit 43eea7f into civicrm:master Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants