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

[Feature] Simple search functionality #9

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

Conversation

FluentCoding
Copy link

The backend PR for this PR: ide-stories/vscode-stories#42

@benawad
Copy link
Contributor

benawad commented Nov 2, 2020

Do you want to do '%' + searchInput + '%' or do you only want exact matches? Also maybe use ilike so it's case insensitive.

@PranjalAgni
Copy link

We can also sanatize query param searchInput, as currently its been directly passed to the native SQL query. What do you think @benawad ?

@benawad
Copy link
Contributor

benawad commented Nov 2, 2020

I believe TypeORM escapes $1 for you

@PranjalAgni
Copy link

Yeah that's there, so it will be done already. Also are we deleting stories after 24 hours or storing it forever

@PranjalAgni
Copy link

I was thinking if we were deleting stories after 24 hours then we don't need pagination as there will be atmost 10 stories available so we can return the stories for that user immediately

@FluentCoding
Copy link
Author

Do you want to do '%' + searchInput + '%' or do you only want exact matches? Also maybe use ilike so it's case insensitive.

My b, didn't think about case insensitivity. Yeah, imo we should also allow "similar" and not only case sensitive exact matches.

@FluentCoding
Copy link
Author

I was thinking if we were deleting stories after 24 hours then we don't need pagination as there will be atmost 10 stories available so we can return the stories for that user immediately

Yh, we would save memory and computation time - also, having an auto deletion with an interval of 1 day seems to be pretty reasonable for stories.

@benawad
Copy link
Contributor

benawad commented Nov 3, 2020

for now I'm keeping all the stories until I wipe the db, but that's a separate concern

@benawad benawad changed the base branch from main to master November 3, 2020 02:31
@PranjalAgni
Copy link

for now I'm keeping all the stories until I wipe the db, but that's a separate concern

Yes I just asked to be sure as in code logic like this was not there

u.flair
from text_story ts
inner join "user" u on u.id = ts."creatorId"
where u.username ILIKE %$1%
Copy link
Member

Choose a reason for hiding this comment

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

Hey, I tried making a request but the %$1% doesn't work for me. I think it's because of the missing apostrophes. Did you try it? @FluentCoding

@Bukii Bukii self-assigned this Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants