EPSCoR / ERCore

ERcore content management system to assist with NSF EPSCoR reporting
4 stars 7 forks source link

Grant & Proposals View bug? #15

Closed khuffman closed 8 years ago

khuffman commented 9 years ago

Isis mentioned during the PA meeting that in the Accomplishment table the Grant & Proposals numbers seem odd , we had also noticed that. For us the Total (Inception through x-date) is correct but the yearly reporting period numbers don't make sense, Sally asked me to look into it. I think I found the error/bug(?) In er_summary_er_proposal view(+ its attachment and export displays) the Proposal view filters by "Awarded" dates. That means also the proposal view's Attachment display is not returning all proposals e.g. denied, submitted, pending since these don't have any "Award dates" set. (awarded date(s) are not required info for adding a proposal).

What is odd is that under the Accomplishment table it says: * Grant/Proposal counts include all grants regardless of status. So why is the proposals view only filtering by "Award date"? was at some point back in time Award date(s) field required for all proposals?

My solution: as a temporarily fix: I changed the View(+attachement+export) not to filter by Award dates, instead it is now filtering by "Submitted date" since all proposals have a "Submitted date". Now each reporting period returns the correct number of "submitted" proposals, regardless of status. I also added a exposed filter type to the proposal views main display, where one can then select which type of proposal to show, e.g. denied or awarded etc.

cjallen-epscor commented 9 years ago

@khuffman - Yes, Isis, Mary Jo and I had a few sessions about this and found this area confusing in ERCore.

We agreed as well that this area should include (and focus) on submitted values and not Awarded dates.

I was on travel and was going to begin working on what we came up with:

 Submitted: All should be counted as submitted
 **Pending: 
 New Local Fix: Submitted - (Awarded + Denied + Expired) = Pending
 Take out Pending button from content type**
aturling commented 9 years ago

Does this fix the issue that was mentioned at the last meeting about Table E (Outputs) - proposals pending - current reporting period and cumulative total are the same number?

Also, maybe related to this, I found a bug that could be affecting the numbers on Table E. I tested it on dev-ercore.nmepscor.net and it appears there too. Here are the steps to recreate it:

  1. Create a proposal, select "pending" as the status, and pick a pending date of notification (required field).
  2. Note that Admin views -> Table E correctly lists this proposal under the "pending" group.
  3. Now edit that proposal and change "pending" to "submitted". The pending date of notification disappears.
  4. Check Admin views -> Table E; that proposal will appear under both "submitted" and "pending" groups when it should only appear under "submitted".

I think this happens because the pending date of notification is still stored in the database even when the status is changed to submitted. This line in er/pages/outputs.inc is selecting on the various fields:

foreach (array("Submitted"=>'field_er_proposal_submit', "Awarded"=>'field_er_proposal_date', "Pending"=>'field_er_proposal_pending') as $label=>$date_field)

Is there a way to change that to query the "status" field (field_er_proposal_status)?

aturling commented 9 years ago

Actually sorry I just reread the comment above and I think removing "pending" as a status that people can select when they add a proposal would fix this error.

cjallen-epscor commented 9 years ago

So - deleting "Pending" seems to be the solution. Here's a simple view showing those 'Pending' nodes - http://dev-ercore.nmepscor.net/proposals-status-pending (export at http://dev-ercore.nmepscor.net/admin/structure/views/view/views_overview/export)

The workflow would be to 1) Find those nodes with pending status 2) Delete the "Pending" date and 3) Change the value something to else (Submitted, Awarded, Expired, Denied) 4) Once all values are assigned to something else visit 'admin/structure/types/manage/er-proposal/fields/field_er_proposal_status' and remove "Pending" value from "field_er_proposal_status"

cjallen-epscor commented 9 years ago

I also found a bug for "Award Amount" being shown/required incorrectly for Awarded/Expired. Patch at https://github.com/EPSCoR/ERCore-3.1/commit/b9fa03fae1d539d44b7aa2bea999696ff6861e9e

aturling commented 9 years ago

I applied the Award Amount patch to our site. Are the award dates supposed to appear when "expired" is chosen as the status?

Also do you have the code fix for pages/outputs.inc to populate pending proposals = submitted - awarded - denied - expired?

cjallen-epscor commented 9 years ago

@aturling - I think you are correct. The award dates are supposed to appear for Expired chosen.

Re: the code fix - I think you are also correct here "Is there a way to change that to query the "status" field (field_er_proposal_status)?" This may need to reference the field_er_proposal_status in respect to the date given.

This 'bug' also applies to the patent CT (attached screenshot). My sample "Webinar" patent gets calculated for all three: awarded, pending, licensed. The user guide and/or author editing this content needs to remember that once their patent has been awarded to remove 'pending' date. screen shot 2015-05-11 at 4 56 01 pm

aturling commented 9 years ago

Here's my first attempt at fixing table E when "pending" is removed as a status. My understanding is that "Submitted" = all proposals, regardless of status, "Awarded" = awarded, and "Pending" = submitted (not awarded, denied, or expired). In that case I joined in the proposal status information and added a check on the status column (according to the rules given in the $status_check array).

This is the modified generate_proposal_date function in pages/outputs.inc:

  private function generate_proposal_data($count, $period){
                $data = array();

                $status_check = array(
                        "Submitted" => array("Submitted", "Awarded", "Expired", "Denied"),
                        "Awarded"   => array("Awarded"),
                        "Pending"   => array("Submitted")
                );

                foreach (array("Submitted"=>'field_er_proposal_submit', "Awarded"=>'field_er_proposal_date', "Pending"=>'field_er_proposal_submit') as $label=>$date_field){
                        $query = db_select('node', 'node');
                        $query->condition("node.type", 'er_proposal', '=');

                        //we need to return the "funds requested" info:
                        $query->innerJoin("field_data_field_er_award_requested_dec", 'amount', 'node.nid = amount.entity_id');
                        if ($count)
                                $query->addExpression('SUM(amount.field_er_award_requested_dec_value)', 'amount');
                        else
                                $query->addField('amount', 'field_er_award_requested_dec_value', 'amount');

                        $query->innerJoin("field_data_{$date_field}", 'date', 'node.nid = date.entity_id');
                        $this->applyDateRange($query, $date_field, $period, $label=="Awarded");//Note: Award date is a date range...

                        // add status
                        $query->innerJoin("field_data_field_er_proposal_status", 'status', 'node.nid = status.entity_id');
                        $query->condition("status.field_er_proposal_status_value", $status_check[$label], 'IN');

                        $data[$label] = $this->generate_node_output($count, $query);
                }
    return $data;
  }

For patents, I'll add help text somewhere on the "create patent" page and mention in the user guide that the user needs to delete the provisional date if/when the patent is awarded.

cjallen-epscor commented 9 years ago

@aturling - awesome! I applied this on my local install and it worked!

cjallen-epscor commented 9 years ago

@aturling - re: Patents - could we add a rule that triggers that "Once patent is added with Provisional date - email author of patent to update their patent once awarded" - kind of like a friendly reminder. Maybe monthly?

@iserna and I discussed having this implemented locally for the Proposals. Notify authors of Proposals monthly if their Proposal is in submission mode (with no other status selected and pending removed) where the email would say something like: "Monthly reminder - you have entered 'Proposal XYZ', if you have an update on this proposal, please update accordingly (Awarded/Denied) at http://url.of.proposal_xyz"

what do you all think?

khuffman commented 9 years ago

Yes, this feature would be nice, Sally also thinks: "..to have a well worded email that reminds participants to update their accounts every quarter.." would be useful as well.

Kia

On 05/14/2015 12:02 PM, Chris Allen wrote:

@aturling https://github.com/aturling - re: Patents - could we add a rule that triggers that "Once patent is added with Provisional date - email author of patent to update their patent once awarded" - kind of like a friendly reminder. Maybe monthly?

@iserna https://github.com/iserna and I discussed having this implemented locally for the Proposals. Notify authors of Proposals monthly if their Proposal is in submission mode (with no other status selected and pending removed) where the email would say something like: "Monthly reminder - you have entered 'Proposal XYZ', if you have an update on this proposal, please update accordingly (Awarded/Denied) at http://url.of.proposal_xyz"

what do you all think?

— Reply to this email directly or view it on GitHub https://github.com/EPSCoR/ERCore-3.1/issues/15#issuecomment-102084937.

iserna commented 9 years ago

@khuffman Chris created a notification for Proposals & Grants. When a participant creates a proposal, six months after it was entered they will get an email/notification reminding them to update the status and I will also get an email so I can make sure it gets updated. I think this is as custom as it gets. So there may be some manual reviewing that needs to be done on behalf of the admin. Does this sound like something Sally would want. Does this sound like something that would be beneficial to all jurisdictions and included in the core?

iserna commented 8 years ago

I am closing this issue and will add a topic for discussion about Notifications to the agenda for the next Tech Meeting (9/9/2015)

khuffman commented 8 years ago

updating this with additional info: https://github.com/EPSCoR/ERCore-3.1/issues/21