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

SirsiDynix Issues: Manual Stats Imports failing #328

Merged
merged 2 commits into from
Apr 6, 2018

Conversation

queryluke
Copy link
Contributor

Problem

The hidden form at the bottom of uploadConfirmation.php does not contain a startDate & numMonths value when manually importing stats file. This caused the scripts in uploadComplete.php(line 38-55) to incorrectly set the $multYear var to true.

Note about solution

To avoid any unnecessary meddling in how uploadConfirmation.php generates its variables, I decided to add additional logic to check for the existence of the startDate. As I understand it, there is a feature request to allow manual multi-year imports, at which point the logic for finding the start/end dates of an import file can be added. But this patch works until that development can be completed.

iconv()

Additionally, it appears that iconv() in the Utility.php class has a (known bug)[http://php.net/manual/en/function.iconv.php#108643], which causes it to return false if it comes across an invalid character. This was cause the confirmation page to error out and not display any data to check. However, if the problem is not global across coral installs, that suggests the bug might be linked to OS and/or php version. Thus, instead of outright replacing iconv(), I've added a check for an empty return, and (if true), it will run the file through mb_convert_encoding() instead.

Review Procedure

Download the following sample files. File > Download as > Tab-separated values. Make sure the files have a .txt extension

JR1 R4 example
BR2 R4 example
BR1 R4 example
DB1 R4 example

OR

Use your own sample reports.

For each test file, in the Usage Module

  1. File Import
  2. Choose the test file
  3. Choose the correct report type
  4. Click 'Upload'
  5. Confirmation should display correctly
  6. Click 'confirm'
  7. Stats should import without error
  8. Click 'Usage Reports'
  9. Select the 'Usage Statistics - Provider Rollup' report and make sure the stats match and/or display without error.

Next, check that the SUSHI functionality didn't break:

  1. Click SUSHI
  2. Click 'Run Now' from an available publisher (selecting any dates), then 'Submit for Processing'
  3. Click 'View to process' in the newly created entry of the Outstanding Import Queue
  4. Click 'Confirm'
  5. The SUSHI report should be processed and added without any errors.

@PaulPoulain PaulPoulain added the bug This is a bug (not an enhancement) label Dec 12, 2017
@cjc66 cjc66 added this to the Version 3.0.0 Beta milestone Mar 22, 2018
@cjc66 cjc66 added the important Not critical but should be addressed label Mar 22, 2018
@jeffnm
Copy link
Contributor

jeffnm commented Apr 6, 2018

This worked fairly well.

I received this database error message when importing the BR2 R4 book sample, but it seemed to work anyway.screen shot 2018-04-06 at 2 14 19 pm

I also had display issues with the Cyrillic characters from the DB1 report. They were fine in the source files, but after upload they were corrupted.
screen shot 2018-04-06 at 2 21 41 pm

I can confirm that I was seeing the iconv() error and this seemed to fix that.

@jeffnm
Copy link
Contributor

jeffnm commented Apr 6, 2018

I just tested a local merge of this PR and #401 and the Cyrillic issue didn't happen, so it may be something on my development server.

@jeffnm jeffnm merged commit 85d47b4 into coral-erm:development Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement) important Not critical but should be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants