-
Notifications
You must be signed in to change notification settings - Fork 215
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
May I suggest a change to respect column name's case? #281
Comments
could you make a pull request? |
Sure. Let me have some time for this and sure. |
Sorry for the delay. Went out on a business trip. We'll try to do this week. :) |
Hi. I'm ready to finally submit my change into a branch for your review. Regards |
why not simply a pull request? |
This would be my first pull request. I read github's documentation and thought it involved me pushing first a branch and from there creating the pull request. Did you mean a pull request from my own clone? Sort of manual pull request? |
yes, a pull request from your fork. |
I don't have a fork in Github. I'm working locally only. But let me try doing a fork :) Hold on. |
That was super easy! Let me push to my fork then. Hold on. |
Sent. Can you see it? |
Tried to create the pull request matching the base fork and origin tag. But couldn't, so I sent it out based on rails3. |
I see it, but I will review it later On Wed, Apr 6, 2016 at 6:41 PM, Alex B. [email protected] wrote:
Best regards, Sent from my Nokia 3310 |
That's fine. As long as you can see it. Thanks! Let me know what you think. |
In file https://github.com/leikind/wice_grid/blob/rails3/lib/wice/columns/column_custom_dropdown.rb
lines 107 and 109, where you generate the table.column_name = something....
I found it useful to chance those lines to:
and
Please notice the "'s surrounding #{@column_wrapper.name}.
I had an issue while using PostgresSQL with a DB where column names had upper cases: with the "'s the name is respected. Without them, it wasn't and PostgreSQL wasn't finding the column, generating an exception and well, crashing the page.
Let me know what you think.
Cheers,
Alex B.
The text was updated successfully, but these errors were encountered: