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.

Sign up for GitHub

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

Jump to bottom

CRM-13283 - Status page project #5973

Closed
wants to merge 50 commits into from
Closed

CRM-13283 - Status page project #5973

wants to merge 50 commits into from

Conversation

MegaphoneJon
Copy link
Contributor

This is the work that Andrew and I worked on during the post-Denver sprint, Michael's work is largely separate.

agh1 and others added 30 commits June 7, 2015 15:14
@agh1 @MegaphoneJon
CRM-13823: combine status messages into a single notification
f1d514d 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@ginkgomzd @MegaphoneJon
add show_hushed option (stub) to api.system.check; CRM-13823
4d925be
@ginkgomzd @MegaphoneJon
alias level as severity for Status paradigm; CRM-13823;
14e55b3
@agh1 @MegaphoneJon
CRM-13823: error message if important alert, alert message otherwise
1b4adaa 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@agh1 @MegaphoneJon
CRM-13823: provide integer value of log levels, filter by that
b1cbb37 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@agh1 @MegaphoneJon
CRM-13823: set severities for existing status checks
c7611fa 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@MegaphoneJon
CRM-13283 - Add schema file for StatusPreference entity
5d67490
@MegaphoneJon
CRM-13283 - Initial BAO for StatusPreference
9294e9d
@MegaphoneJon
CRM-13283 - Fix copyright notice on BAO/StatusPreference.php
bd5049b
@MegaphoneJon
CRM-13283 - Initial API for StatusPreference
f3248c0
@MegaphoneJon
CRM-13283 - Update StatusPreference schema to fix bugs
0ebeac8
@MegaphoneJon
CRM-13283 - set Status Check Name as required on create
67b3c19
@agh1 @MegaphoneJon
CRM-13823 Move dashboard notices to system check
9df1e44 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@agh1 @MegaphoneJon
CRM-13823 Sort status messages by severity, name - even in API
47ca09f 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@MegaphoneJon
Add check_info to StatusPreference entity
699df5e
@MegaphoneJon
CRM-13283 - minimum_report_severity defaults to 1
14b1c57
@agh1 @MegaphoneJon
CRM-13823 find status preference by name/domain combo
3e94a16 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@MegaphoneJon
CRM-13283 - First set of unit tests
ca0f25a
@agh1 @MegaphoneJon
CRM-13823 Status check for last cron run
b77f792 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@agh1 @MegaphoneJon
CRM-13823 Handle situation when cron hasn't run at all
8b9efda 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@MegaphoneJon
CRM-13283 - Initial StatusPreference API unit tests
ae361f3
@MegaphoneJon
CRM-13283 - ensure minimum_report_severity can't exceed 8
3dbd54b
@MegaphoneJon
Enforce maximum value of 8 for minimum_report_severity; first crack a…
cc9378b 
…t allowing strings via API
@MegaphoneJon
CRM-13283 - lowercase minimum_report_severity strings
ecbe730
@MegaphoneJon
CRM-13283 - add API unit tests to StatusPreference to support passing…
494e5dd 
… a string as a minimum_report_severity
@MegaphoneJon
CRM-13283 - Remove debug statement
fde6d11
@MegaphoneJon
CRM-13283 - Edit StatusPreference API to accept a string for minimum_…
89baee7 
…report_severity
@MegaphoneJon
CRM-13283 - implement string to integer conversion; sanity checks on …
07267a8 
…valid severity values
@MegaphoneJon
CRM-13283 - Initial SystemCheckTest.php
6ceec14
@MegaphoneJon
CRM-13283 - Initial work on date-sensitive time checks
15ae0db
agh1 added 2 commits June 7, 2015 15:24
@agh1 @MegaphoneJon
CRM-13823 check extensions. DB version, DB storage engine
9d04fa8 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@agh1 @MegaphoneJon
CRM-13823 better messages for directory checks
736f7be 
----------------------------------------
* CRM-13823: Admin Status Page
  https://issues.civicrm.org/jira/browse/CRM-13823
@colemanw
Copy link
Member

colemanw commented Jun 7, 2015

Wait, no bomb icon? How are we supposed to know what's going on with CiviCRM without a bomb icon??

@eileenmcnaughton
Copy link
Contributor

Looks like Jenkins has something to say.... but I think despite the claim of 45 alerts a lot of them are in a single block of code

@MegaphoneJon
Copy link
Contributor Author

This looks to mostly be civilint stuff...I didn't civilint others' code, and by the look of it, maybe didn't even civilint all of mine. I'll go over that tomorrow.

@agh1
Copy link
Contributor

agh1 commented Jun 8, 2015

I think I'm the culprit on most of it--spacing issue here and there. I'll take care of it in a minute.

@agh1
CRM-13283 misc sp acing problems
c6eacb3 
----------------------------------------
* CRM-13283: Number of terms does not update the end date of membership
  https://issues.civicrm.org/jira/browse/CRM-13283
@agh1
Copy link
Contributor

agh1 commented Jun 8, 2015

Okay so I created MegaphoneJon#1 against PalanteJon/status-page-jon which should fix the problems. And yes, I was responsible for 90% of the problems (I was putting spaces after function comments all over the place).

@MegaphoneJon
Merge pull request #1 from agh1/status-page-2
db4168c 
CRM-13823 misc sp  acing    problems
@MegaphoneJon
Copy link
Contributor Author

Just a note that this should be CRM-13823, not CRM-13283.

@eileenmcnaughton
Copy link
Contributor

still red :-(

@agh1
Copy link
Contributor

agh1 commented Jun 8, 2015

These are a bunch of test failures--some of the new api, and others that are new tests themselves. @ginkgomzd @PalanteJon you know the new API code better than I.

@colemanw here you go:
bomb

@colemanw
Copy link
Member

colemanw commented Jun 8, 2015

👍
Ooh, github comments have lots of emoticons. How about:
💣

@eileenmcnaughton
Copy link
Contributor

looks like they are mostly e-notices

@eileenmcnaughton
Copy link
Contributor

Looks like an integer rather than a string is being passed to the severity map function - leading to enotice - leading to bomb

@MegaphoneJon
Copy link
Contributor Author

This is on my radar for this week, I haven't forgotten! The severity map should be able to take integer OR string, depending on the value of the $reverse argument. I'll get this sorted.

@eileenmcnaughton
Copy link
Contributor

How goes on this?

@MegaphoneJon
Copy link
Contributor Author

At the top of my list for Wednesday.

@MegaphoneJon
Copy link
Contributor Author

Eileen, Andrew - OK, I took a crack at this today. Andrew, I think you're more familiar with the code in question, so I'd like you to review. My first commit removed a few calls to severityMap where we were already evaluating integers. There's a second commit I did NOT make - look at $returnValues here: https://github.com/PalanteJon/civicrm-core/blob/status-page-jon/CRM/Utils/Check/Env.php#L418.

It seems like that array isn't used anywhere - and there's an e-notice because $return isn't defined. I'm not sure what this code is intended to do, but I wanted to check in with you before simply removing it.

Finally - when I clicked on "Details" on my failed build merge above, I got a 404 error. Do the test reports expire?

@eileenmcnaughton
Copy link
Contributor

They do expire - but you have a new result now due to your new commit. Sadly it's rejected you on test formatting

@MegaphoneJon
Copy link
Contributor Author

Ugh, the one day my linter isn't working...but that's OK, I'm just
excited to have a new test result. I'll check this out today.

@MegaphoneJon
Copy link
Contributor Author

Hmm - these Jenkins results all relate to files that aren't touched by this PR. It points to formatting issues with:
ActivitySearch.php
ContribSYBNT.php
EventAggregate.php
TagContributions.php
FourFive.php
AddMessageTemplateTest.php

None of those are the 24 files we edited: https://github.com/civicrm/civicrm-core/pull/5973/files

Does that mean we're off the hook?

@totten
Copy link
Member

totten commented Jul 8, 2015

Spot checking a random file, the smelly code in CRM/Contact/Form/Search/Custom/ContribSYBNT.php appears to have originated circa March 1 (8bb3a4f) and it was cleaned up circa May 14 (d14ccbd). It doesn't have anything to do with this PR, but I suspect that the PR hasn't been rebased or merged since early May (and thus never got the cleanup). Tangentially: Github says it can't merge the branch automatically due to conflicts. If it were my branch, I'd rebase and resolve conflicts.

@MegaphoneJon
Copy link
Contributor Author

I can take a hint :) I'll try not to let this sit too long.

@MegaphoneJon
Copy link
Contributor Author

Hmm...I rebased but I'm still getting errors? I'm going to ask Joseph for some git help.

@MegaphoneJon MegaphoneJon mentioned this pull request Jul 21, 2015
Merged
@MegaphoneJon
Copy link
Contributor Author

Closing in favor of #6276.

@MegaphoneJon MegaphoneJon deleted the status-page-jon branch October 10, 2017 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MegaphoneJon @colemanw @eileenmcnaughton @agh1 @totten @ginkgomzd