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

Search by code #598

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Search by code #598

wants to merge 3 commits into from

Conversation

frankete333
Copy link
Member

@frankete333 frankete333 commented Nov 20, 2024

Add the ability to search a subject by the code
Also, making the X in the search bar clears the text input

Copy link
Collaborator

@santiagorodriguez96 santiagorodriguez96 left a comment

Choose a reason for hiding this comment

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

Bien metidaaaaa. Dejo un par de comentarios 🙂

@@ -18,6 +18,7 @@ def all
Subject
.where("lower(unaccent(name)) LIKE lower(unaccent(?))", "%#{params[:search].strip}%")
.or(Subject.where("lower(unaccent(short_name)) LIKE lower(unaccent(?))", "%#{params[:search].strip}%"))
.or(Subject.where("lower(unaccent(code)) LIKE lower(unaccent(?))", "%#{params[:search].strip}%"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Es necesario el unaccent si estamos usando el código para buscar? Entiendo que el lower si así tanto si buscás por 'mi2' o por 'MI2' vas a encontrar a Matemática inicial

@@ -71,7 +71,7 @@
<%= form_with(url: all_subjects_path, method: :get, local: true) do |f| %>
<div class="mdc-text-field mdc-text-field--filled mdc-text-field--label-floating mdc-text-field--no-label mdc-text-field--with-trailing-icon">
<span class="mdc-text-field__ripple"></span>
<%= f.text_field :search, value: params[:search], class: "mdc-text-field__input", placeholder: "Buscar", autofocus: true, data: { 'search-target': "searchInput" } %>
<%= f.text_field :search, value: params[:search], class: "mdc-text-field__input", placeholder: "Busqueda por nombre o codigo de la materia", autofocus: true, data: { 'search-target': "searchInput" } %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo:

Suggested change
<%= f.text_field :search, value: params[:search], class: "mdc-text-field__input", placeholder: "Busqueda por nombre o codigo de la materia", autofocus: true, data: { 'search-target': "searchInput" } %>
<%= f.text_field :search, value: params[:search], class: "mdc-text-field__input", placeholder: "Búsqueda por nombre o codigo de la materia", autofocus: true, data: { 'search-target': "searchInput" } %>

Aunque creo que me gusta un poco más el verbo en infinitivo:

Suggested change
<%= f.text_field :search, value: params[:search], class: "mdc-text-field__input", placeholder: "Busqueda por nombre o codigo de la materia", autofocus: true, data: { 'search-target': "searchInput" } %>
<%= f.text_field :search, value: params[:search], class: "mdc-text-field__input", placeholder: "Buscar por nombre o código de la materia", autofocus: true, data: { 'search-target': "searchInput" } %>o

Copy link
Collaborator

Choose a reason for hiding this comment

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

Tenemos un par de tests de sistema que testean medio lo que estamos testeando acá. Por ejemplo uno que testea que se oculten y se muestren las materias correctas y otro que testea la funcionalidad de buscar materias.

Qué te parece simplemente añadir un par de expectations más al último para testear que buscar por código anda y después vemos de mover esos tests de Minitest a RSpec como follow up?

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.

2 participants