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

extra '' causing error on calendar consolidated request tab #653

Merged
merged 2 commits into from Sep 12, 2016

Conversation

ying-pbrc
Copy link
Collaborator

sr error consolidated request tab

@jleonardw9
Copy link
Contributor

Looks good, thanks.

@jayhardee9
Copy link
Contributor

Good catch! Can you also fix line 161 in the same file?

@ying-pbrc
Copy link
Collaborator Author

good catch, didn't see that one. its updated now.

@jayhardee9
Copy link
Contributor

One more thing! It seems that the changes you made are hiding draft services on the Step 2B calendar. If you only compare the SubServiceRequest status against 'first_draft' only (in the two lines you changed), I believe you can fix your error while preserving existing functionality. Sorry for any confusion!

@ying-pbrc
Copy link
Collaborator Author

Actually, i am still running on OS v1.7.5, that was probably why i didn't see the 2nd %w('first_draft' 'draft') on line 161 at the first place. In v1.7.5. I don't think i have the most current version of your master branch. I will close this pull request, and do new pull request then submit my fix.

@ying-pbrc ying-pbrc closed this Aug 31, 2016
@ying-pbrc
Copy link
Collaborator Author

sorry Jay,
The branch that i submitted my fix was based off your current master branch - i had too many different versions running on my machine. Since I have no knowledge of any functionality changes in development, i don't want to make any logic change to OS code. This pull request is only meant to fix bug that caused by the unintended typos.

@ying-pbrc ying-pbrc reopened this Aug 31, 2016
@ying-pbrc
Copy link
Collaborator Author

please review. Thanks

@jayhardee9
Copy link
Contributor

I understand that was not your intention! It looks good to me now.

@jleonardw9
Copy link
Contributor

Ying, this looks great! Thanks for catching this, we'll get you pulled in as soon as the team goes over pull requests. We've got a few production issues we're trying to settle at the moment, so we're on hold for now.

@ying-pbrc
Copy link
Collaborator Author

No problems. Thanks for the update.

From: Jason Leonard [mailto:notifications@github.com]
Sent: Wednesday, August 31, 2016 12:40 PM
To: sparc-request/sparc-request sparc-request@noreply.github.com
Cc: Ying Wu Ying.Wu@pbrc.edu; State change state_change@noreply.github.com
Subject: Re: [sparc-request/sparc-request] extra '' causing error on calendar consolidated request tab (#653)

Ying, this looks great! Thanks for catching this, we'll get you pulled in as soon as the team goes over pull requests. We've got a few production issues we're trying to settle at the moment, so we're on hold for now.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHubhttps://github.com//pull/653#issuecomment-243841882, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHS1u2FgcJizKF5Q85ZHZ5Ne1vljhHxRks5qlbx6gaJpZM4JuT6c.

@jleonardw9 jleonardw9 merged commit beb3ebc into sparc-request:master Sep 12, 2016
@ying-pbrc ying-pbrc deleted the merged-calendar-error branch February 13, 2017 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants