search

leikind / wice_grid

A Rails grid plugin to create grids with sorting, pagination, and (automatically generated) filters
MIT License
537 stars 215 forks source link

May I suggest a change to respect column name's case? #281

Open ablancag opened 8 years ago

ablancag commented 8 years ago

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:

" #{@column_wrapper.alias_or_table_name(table_alias)}.\"#{@column_wrapper.name}\" is " + opts

and 

[" #{@column_wrapper.alias_or_table_name(table_alias)}.\"#{@column_wrapper.name}\" = ?", opts]

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.

leikind commented 8 years ago

could you make a pull request?

ablancag commented 8 years ago

Sure. Let me have some time for this and sure.

ablancag commented 8 years ago

Sorry for the delay. Went out on a business trip.

We'll try to do this week. :)

ablancag commented 8 years ago

Hi. I'm ready to finally submit my change into a branch for your review. Can you give me permissions to push my branch?

Regards

leikind commented 8 years ago

why not simply a pull request?

ablancag commented 8 years ago

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?

leikind commented 8 years ago

yes, a pull request from your fork. Github makes it very easy to do

ablancag commented 8 years ago

I don't have a fork in Github. I'm working locally only. But let me try doing a fork :) Hold on.

ablancag commented 8 years ago

That was super easy! Let me push to my fork then. Hold on.

ablancag commented 8 years ago

Sent. Can you see it?

ablancag commented 8 years ago

Tried to create the pull request matching the base fork and origin tag. But couldn't, so I sent it out based on rails3.

leikind commented 8 years ago

I see it, but I will review it later

On Wed, Apr 6, 2016 at 6:41 PM, Alex B. notifications@github.com wrote:

Tried to create the pull request matching the base fork and origin tag. But couldn't, so I sent it out based on rails3.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/leikind/wice_grid/issues/281#issuecomment-206457628

Best regards, Yuri Leikind

Sent from my Nokia 3310

ablancag commented 8 years ago

That's fine. As long as you can see it. Thanks! Let me know what you think.