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

Added search feature on Donations and Needs page #183

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

Conversation

nadeeravista
Copy link

What you've done

Added search feature on donations and needs page.
Also created a settings file which we can use to store application related settings such as pagination etc.

Why have you done it

Screenshot (if visual)

...

Any testing carried out

Locally tested with English and Sinhala

@@ -81,7 +81,7 @@ public function searchDonations($search){
$query = $query->where(DB::Raw('CONCAT_WS(" ",name,telephone,address,city,donation,information) '),"like","%$searchPart%");
}
}
return $query->get();
return $query->orderBy('id', 'desc')->paginate(\Config::get('rf_settings.donations_page_pagination'));
Copy link
Contributor

Choose a reason for hiding this comment

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

\Config::get can be replaced with config() calls

Copy link
Contributor

@gayanhewa gayanhewa left a comment

Choose a reason for hiding this comment

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

Apart from the requested changes, the rest looks good. Once updated I will run a manual test.

@@ -83,7 +83,7 @@ public function searchNeeds($search){
$query = $query->where(DB::Raw('CONCAT_WS(" ",name,telephone,address,city,needs)'),"like","%$searchPart%");
Copy link
Contributor

Choose a reason for hiding this comment

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

This query might be open to SQL injection. Since the calls to searchNeeds with the parameter is passed to a DB::raw which doesn't use prepared statements nor sanitises the input.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nadeeravista could you please have a look on @gayanhewa's comment as well as fix conflicts?

@heimdallrj heimdallrj changed the base branch from master to dev June 7, 2017 20:43
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.

3 participants