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

Assume unknown db column types are un-searchable #193

Merged
merged 1 commit into from
Nov 9, 2015

Conversation

c-lliope
Copy link
Contributor

@c-lliope c-lliope commented Nov 8, 2015

Closes #178

Problem:

If users have a database set up with unknown column types,
Administrate treats those columns as a string.
This can cause bugs when users try to search,
because the unknown database column might not support
the string search operator.

Solution:

  • Add a searchable option to all field types,
    which specifies whether it is one of
    the resource's plaintext search fields.
  • For unknown db column types, give the field
    a default option of searchable: false.


expect(dashboard).to contain("name: Field::String,")
expect(dashboard).to contain("email: Field::String,")
expect(attrs[:ip_address]).to eq(Administrate::Field::String.with_options(searchable: false))

Choose a reason for hiding this comment

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

Line is too long. [103/80]

@why-el
Copy link

why-el commented Nov 9, 2015

Yep, we faced this just a moment ago. Good work.

@c-lliope c-lliope added this to the 0.1.1 milestone Nov 9, 2015
@tute
Copy link
Contributor

tute commented Nov 9, 2015

This LGTM. 🚀

Closes #178

Problem:

If users have a database set up with unknown column types,
Administrate treats those columns as a string.
This can cause bugs when users try to search,
because the unknown database column might not support
the string search operator.

Solution:

- Add a `searchable` option to all field types,
  which specifies whether it is one of
  the resource's plaintext search fields.
- For unknown db column types, give the field
  a default option of `searchable: false`.
@c-lliope c-lliope merged commit ae5f3ba into master Nov 9, 2015
@c-lliope c-lliope deleted the gw-default-field branch November 9, 2015 18:34
@c-lliope c-lliope removed the In Review label Nov 9, 2015
c-lliope added a commit that referenced this pull request Nov 12, 2015
- Update gem version
- Update README with recommended optimistic versioning for bundler
- Update README with warning about Administrate's pre-1.0 status
- Update CHANGELOG to fill in missing PR references
- sort CHANGELOG entries according to change type

Changes:

```
* [#191] [CHANGE] Improve API for specifying how resources are displayed
  across the dashboard.
  * Models are now displayed with a sensible default - (e.g. "User #2")
  * Users can define `ModelDashboard#display_resource(resource)` for custom
    display behavior
  * Users who have generated views for the following field types
    may need to update them to take advantage of the new API:
    * HasOne
    * HasMany
    * Polymorphic
    * BelongsTo
* [#223] [FEATURE] Translation: Vietnamese
* [#161] [FEATURE] Translation: Mandarin Chinese
* [#196] [FEATURE] Translation: Taiwanese Mandarin
* [#142] [FEATURE] Translation: Brazilian Portuguese
* [#171] [FEATURE] Translation: Polish
* [#153] [FEATURE] Translation: Russian
* [#148] [FEATURE] Translation: French
* [#147] [FEATURE] Translation: German
* [#154] [FEATURE] Translation: Spanish
* [#126] [UI] Preserve whitespace when rendering text fields
* [#194] [BUGFIX] Don't clear out datetime values in form fields
* [#193] [BUGFIX] Don't assume that unrecognized db column types are searchable
* [#124] [BUGFIX] Better detection of application models
* [#156] [COMPAT] Include missing `sass-rails` dependency in gemspec
* [#174] [COMPAT] Make several missing dependencies explicit.
* [#144] [COMPAT] Update repository structure so Bundler can pull the gem from github.
  (e.g. `gem "administrate", github: "thoughtbot/administrate"`)
* [#166] [COMPAT] Use ANSI SQL standards for case-insensitive search
* [#120] [DOC] Add Rubygems version badge to README
* [#165] [DOC] Add CircleCI badge to README
* [#119] [DOC] Add CodeClimate badge to README
```
c-lliope added a commit that referenced this pull request Nov 12, 2015
- Update gem version
- Update README with recommended optimistic versioning for bundler
- Update README with warning about Administrate's pre-1.0 status
- Update CHANGELOG to fill in missing PR references
- add `[I18n]` category to CHANGELOG
- sort CHANGELOG entries according to change type

Changes:

* [#191] [CHANGE] Improve API for specifying how resources are displayed
  across the dashboard.
  * Models are now displayed with a sensible default - (e.g. "User #2")
  * Users can define `ModelDashboard#display_resource(resource)` for custom
    display behavior
  * Users who have generated views for the following field types
    may need to update them to take advantage of the new API:
    * HasOne
    * HasMany
    * Polymorphic
    * BelongsTo
* [#223] [FEATURE] Translation: Vietnamese
* [#161] [FEATURE] Translation: Mandarin Chinese
* [#196] [FEATURE] Translation: Taiwanese Mandarin
* [#142] [FEATURE] Translation: Brazilian Portuguese
* [#171] [FEATURE] Translation: Polish
* [#153] [FEATURE] Translation: Russian
* [#148] [FEATURE] Translation: French
* [#147] [FEATURE] Translation: German
* [#154] [FEATURE] Translation: Spanish
* [#126] [UI] Preserve whitespace when rendering text fields
* [#194] [BUGFIX] Don't clear out datetime values in form fields
* [#193] [BUGFIX] Don't assume that unrecognized db column types are searchable
* [#124] [BUGFIX] Better detection of application models
* [#156] [COMPAT] Include missing `sass-rails` dependency in gemspec
* [#174] [COMPAT] Make several missing dependencies explicit.
* [#144] [COMPAT] Update repository structure so Bundler can pull the gem from github.
  (e.g. `gem "administrate", github: "thoughtbot/administrate"`)
* [#166] [COMPAT] Use ANSI SQL standards for case-insensitive search
* [#120] [DOC] Add Rubygems version badge to README
* [#165] [DOC] Add CircleCI badge to README
* [#119] [DOC] Add CodeClimate badge to README
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.

4 participants