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

69 renewals error msgs #80

Merged
merged 20 commits into from
Jan 27, 2020
Merged

69 renewals error msgs #80

merged 20 commits into from
Jan 27, 2020

Conversation

banukutlu
Copy link
Contributor

@banukutlu banukutlu commented Jan 24, 2020 •
edited
Loading

Issue #69

Additional Changes

Refactoring to use bib_summary, added Rails.logger for failed renewals, handled an error when no items selected to renew

@banukutlu banukutlu added this to the 0.1.x Pre Release milestone Jan 24, 2020
@banukutlu banukutlu requested a review from cdmo January 24, 2020 20:26
@banukutlu banukutlu self-assigned this Jan 24, 2020
@banukutlu banukutlu marked this pull request as ready for review January 24, 2020 21:48
cdmo
cdmo suggested changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@cdmo cdmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finished going through it, just a couple of ideas to consider for edits.

Banu Hapeloglu Kutlu and others added 8 commits January 27, 2020 10:42
@cdmo
Update spec/services/symphony_client_spec.rb
681aecd 
Co-Authored-By: Charlie Morris <523381+cdmo@users.noreply.github.com>
@cdmo
Update app/controllers/renewals_controller.rb
8ff263f 
Co-Authored-By: Charlie Morris <523381+cdmo@users.noreply.github.com>
@cdmo
refactoring bulk_renewal_flash using @cdmo 's suggestion
1f99763 
Co-Authored-By: Charlie Morris <523381+cdmo@users.noreply.github.com>
Merge branch '69-renewals-error-msgs' of github.com:psu-libraries/mya…
0d1b93f 
…ccount into 69-renewals-error-msgs
@cdmo
refactoring and renaming renewal_prompt @cdmo 's suggestion
043c653 
Co-Authored-By: Charlie Morris <523381+cdmo@users.noreply.github.com>
changing to span
3aae0d5
refactoring failed renewal response as a hash, by @cdmo's suggestion
344d997
changing include to eq per @cdmo's suggestion
a74cb34
@banukutlu banukutlu requested a review from cdmo January 27, 2020 18:12
@cdmo
Refactored to make the renewal response from Sirsi uniform. (#84)
ea685b6 
* Refactored to make the renewal response from Sirsi uniform.
* Swap "error_message" with "sirsi_response"

Co-authored-by: Banu Hapeloglu Kutlu <bzk60@psu.edu>
cdmo
cdmo approved these changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@cdmo cdmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙆‍♂

@cdmo cdmo merged commit 216cdd5 into master Jan 27, 2020
@cdmo cdmo deleted the 69-renewals-error-msgs branch January 27, 2020 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdmo cdmo cdmo approved these changes

Assignees

@banukutlu banukutlu

Labels
None yet
Projects
None yet
Milestone
0.1.x Pre Release
Development

Successfully merging this pull request may close these issues.
2 participants
@banukutlu @cdmo