Show search results 882

RivaD2 commented 2 years ago

Shows the book search results we get from our Books API (GraphQL server). For now, it displays 8 of the same book and uses same image for all of the search results along with the author and title. If no books are found, results not found image is displayed

Resources used:

Files changed:

TO FIX: Shelf-Modal.tsx:

Comments out Alert component import from Material UI that is crashing the app/won't allow me to run the app. It has to do with the component being part of the lab and not material ui core.
Comments out lines 126- 142 and replaces Alert with <div> temporarily until we can fix the changes that were made using Alert component
This is where the impacting change came from: https://github.com/Project-Books/book-project/pull/892

Search.css

Adds styles for input, search button and icon
Adds styles for no books found image
Uses Grid layout to display 8 books, 2 rows of 4
Shows books with background color shown in issue
Renders results-not-found-container when the data is not null and contains no results from the query

SearchResults.tsx:

Uses useLazyQuery hook to execute our query in response to click event (when user clicks on search icon)
Holds state for searchTerm entered in by user
Holds state for reformattedQueryData: NOTE: I created this additional state variable because the initial data returned from the query was blocking me due to its structure. The data coming back ideally would be an array because we are expecting a list of items but the data takes this shape currently:
Uses useEffect hook to handle data
Holds gql query that is passed to the useLazyQuery hook

SearchResults.tsx:

Defines interface ISearchResultProps
Defines interface IQueryResult
Maps over query data from props and displays book image, title, author currently (renders 8 of same book for now as discussed (we will need to add more books into db. Currently user searches only by title or author, and we have only 4 books in the db, all of which are different so the query can't return more than 1 book per title entered).

src

Adds image folder and no books found image to project

HttpClient.ts

added fix for breaking linter rule of Rule: "@typescript-eslint/no-explicit-any" found on main
type HttpResponse expected <Response>

Additional context

Results found after query: Books scale in size (images taken from laptop, but I did test on my larger monitor)

Results not found on Search

it' done :)

Related issue

Closes #882

Pull request checklist

Please keep this checklist in & ensure you have done the following:

[x] Read, understood and adhered to our contributing document.
- [ x] Ensure that you were first assigned to a relevant issue before creating this pull request
- [] Ensure code changes pass all tests
[x] Read, understood and adhered to our style guide. A lot of our code reviews are spent on ensuring compliance with our style guide, so it would save a lot of time if this was adhered to from the outset.
[x] Filled in the summary, context (if applicable) and related issue section. Replace the square brackets and its placeholder content with your contents. For an example, see any merged in pull request
[x] Set this pull request to 'draft' if you are still working on it
[x] Resolved any merge conflicts

For any of the optional checkboxes (e.g. the screenshots one), still check it if it does not apply.

knjk04 commented 2 years ago

It looks like the merge from main into your branch may not have worked. I can see the MySQL Docker image specified, but we are now using PostgreSQL.

I've just merged in a new change which changes how you'll run the backend. You won't need to run ./mvnw clean install anymore, so check out the new instructions in the README. Please could you also ensure that you have incorporated the latest changes from the main branch into your feature branch?

I'm hoping this new change works, so let me know if you encounter any issues!

RivaD2 commented 2 years ago

@knjk04 My merge did clear( I use sourceTree so I can see clearly where my merge was successful yesterday and today after I pulled in recent changes). Not sure what what happened though 👍

I followed the updated instructions in the readme and have stopped all containers but am getting this error regarding other ports in use when I run Run docker-compose up to start the containers :

Currently I see in Docker:

books-api (not running)
Backend (not running, says exited undefined
book-app (not running)
book-project (not running, I am going to remove this container since we have renamed it)

After pulling in recent changes, my yml file's ports for frontend are listed as: 3000:3000 Ports listed for postgres DB are listed as: - 5433:5432 Ports for backend/container are: 8080:8080

RivaD2 commented 2 years ago

@knjk04 Disregard previous message, I got app and running 👍 I had to remove the old container and then I restarted then stopped everything again and restarted and it worked haha.

RivaD2 commented 2 years ago

@knjk04 Hi there Karan!

Quick update here, this is taking me a bit longer than expected. Currently, the useQuery hook is returning the data after I enter in a search term and submit which is what we want. I also have 8 books rendering (title, author, id from the data and image used for the book).

However, I have run into a bit of a snag with the data because the query expects the search term to be present (and for us, this makes sense because we don't want books to show upon initial render, only after a user searches for a book).

Without a search term entered and search submitted, data is null and errors are thrown on page load (my mapping over the data throws the errors because nothing from search bar has been submitted, and no data has come back from query.) I have to comment out everything in render, enter the search term in search bar and submit, THEN uncomment out my mapping() of data, and I see all books/data as expected.

I thought about storing the data in state, but the useQuery hook provides the data which is already available for use. I just wanted to give you an update 👍

I am working on the best approach currently to fix, looking into what piece I am missing.

knjk04 commented 2 years ago

Hi @RivaD2, thanks for the update! Just to clarify, it would be good if we only call the Books API when the user presses enter after entering in a search query

RivaD2 commented 2 years ago

Hi @RivaD2, thanks for the update! Just to clarify, it would be good if we only call the Books API when the user presses enter after entering in a search query

Roger that, That is what I am working to fix now :) 👍

I believe the errors are thrown currently because the query is executing immediately, but no search term is passed in (The good news is, is that when we pass a search term in and press enter, it works just fine).

I found out that I can use useLazyQuery instead of useQuery to fix. useLazyQuery does not execute the query on mounting of component, but instead waits for an event to trigger the query 👍

RivaD2 commented 2 years ago

@knjk04 I pulled in all recent changes for books-api and book-project. I followed instructions for running books-api but I keep getting exited with code 1. I see the following:

Can you provide any guidance on how to move past this? You mentioned some command for flyway errors previously (./mvnw flyway:clean but that doesn't work in this directory. Is there some other command you know that I could try for FlyWayExceptions if that is in fact the cause of the above?).

Thank you 👍

knjk04 commented 2 years ago

Hi Riva, the flyway clean command should have worked. I know you said it doesn't work in the directory you're in, but what happens when you try?

RivaD2 commented 2 years ago

@knjk04

Hi there Karan, When I tried running the flyway clean command, I received: Permission denied.

I was able to execute the flyway clean command and now all is ok. I had to make the maven file executable first.

knjk04 commented 2 years ago

@RivaD2 The frontend is failing to compile for me due to a linter error:

book_project_frontend  | Failed to compile.
book_project_frontend  |
book_project_frontend  | Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.
book_project_frontend  | Occurred while linting /app/src/shared/http/HttpClient.ts:40
book_project_frontend  | Rule: "@typescript-eslint/no-explicit-any"
book_project_frontend  | assets by path static/ 5.13 MiB
book_project_frontend  |   assets by path static/js/*.js 5.08 MiB
book_project_frontend  |     asset static/js/bundle.js 5.07 MiB [emitted] (name: main) 1 related asset
book_project_frontend  |     asset static/js/node_modules_web-vitals_dist_web-vitals_es5_min_js.chunk.js 5.36 KiB [emitted] 1 related asset
book_project_frontend  |   assets by path static/media/*.png 54.8 KiB
book_project_frontend  |     asset static/media/logo_one_line@1x.23f916ea16ac8c2c578e.png 36.9 KiB [emitted] [immutable] [from: src/shared/media/logo/logo_one_line@1x.png] (auxiliary name: main)
book_project_frontend  |     asset static/media/logo-two-lines-white@1x.2f54409101dcc77671a2.png 17.9 KiB [emitted] [immutable] [from: src/shared/media/logo/logo-two-lines-white@1x.png] (auxiliary name: main)
book_project_frontend  | asset index.html 2.82 KiB [emitted]
book_project_frontend  | asset asset-manifest.json 704 bytes [emitted]
book_project_frontend  | orphan modules 4.03 MiB [orphan] 5991 modules
book_project_frontend  | runtime modules 31.4 KiB 15 modules
book_project_frontend  | cacheable modules 4.29 MiB (javascript) 54.8 KiB (asset)
book_project_frontend  |   modules by path ./node_modules/ 3.99 MiB 401 modules
book_project_frontend  |   modules by path ./src/ 299 KiB (javascript) 54.8 KiB (asset)
book_project_frontend  |     javascript modules 285 KiB 63 modules
book_project_frontend  |     asset modules 13.9 KiB (javascript) 54.8 KiB (asset)
book_project_frontend  |       ./src/shared/media/logo/logo_one_line@1x.png 42 bytes (javascript) 36.9 KiB (asset) [built] [code generated]
book_project_frontend  |       + 3 modules
book_project_frontend  |   modules by mime type image/svg+xml 4.4 KiB
book_project_frontend  |     data:image/svg+xml,%3csvg xmlns=%27.. 281 bytes [built] [code generated]
book_project_frontend  |     data:image/svg+xml,%3csvg xmlns=%27.. 279 bytes [built] [code generated]
book_project_frontend  |     data:image/svg+xml,%3csvg xmlns=%27.. 161 bytes [built] [code generated]
book_project_frontend  |     data:image/svg+xml,%3csvg xmlns=%27.. 271 bytes [built] [code generated]
book_project_frontend  |     + 12 modules
book_project_frontend  |
book_project_frontend  | ERROR in Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.
book_project_frontend  | Occurred while linting /app/src/shared/http/HttpClient.ts:40
book_project_frontend  | Rule: "@typescript-eslint/no-explicit-any"
book_project_frontend  |
book_project_frontend  | webpack 5.69.1 compiled with 1 error in 59078 ms
book_project_frontend  | No issues found.
book_project_frontend  | Compiling...
book_project_frontend  | Compiled successfully!
book_project_frontend  | assets by status 60.2 KiB [cached] 3 assets
book_project_frontend  | assets by status 5.08 MiB [emitted]
book_project_frontend  |   assets by chunk 5.07 MiB (name: main)
book_project_frontend  |     asset static/js/bundle.js 5.07 MiB [emitted] (name: main) 1 related asset
book_project_frontend  |     asset main.67655b5faf3fd23871bc.hot-update.js 363 bytes [emitted] [immutable] [hmr] (name: main) 1 related asset
book_project_frontend  |   assets by path *.json 857 bytes
book_project_frontend  |     asset asset-manifest.json 829 bytes [emitted]
book_project_frontend  |     asset main.67655b5faf3fd23871bc.hot-update.json 28 bytes [emitted] [immutable] [hmr]
book_project_frontend  |   asset index.html 2.82 KiB [emitted]
book_project_frontend  | Entrypoint main 5.07 MiB (4.85 MiB) = static/js/bundle.js 5.07 MiB main.67655b5faf3fd23871bc.hot-update.js 363 bytes 4 auxiliary assets
book_project_frontend  | cached modules 8.32 MiB (javascript) 54.8 KiB (asset) [cached] 6475 modules
book_project_frontend  | runtime modules 31.4 KiB 15 modules
book_project_frontend  | webpack 5.69.1 compiled successfully in 1659 ms
book_project_frontend  | No issues found.

This is when I run it via Docker. Do you see the same error locally when you run the app with Docker?

knjk04 commented 2 years ago

Also, when I don't have the Books API running, I see the following screen:

This is after I typed in a search query ('A brief history of time') and pressed enter. I then saw a blank white page with 'loading' text before seeing the above error.

Would it be possible to show an error message on the same page as the one with the search bar when the Books API is down?

Thanks!

RivaD2 commented 2 years ago

@knjk04 Thank you for your review!

The first screenshot appears to be telling me that there is a linter error in HttpClient (the linter does not like any). I do not see this on my end and was able to see/run the app fine previously before I created this PR. Not sure what changed here since I didn't make any changes in file and linter is passing checks on my end (it says, "All checks have passed below"). I will review file/ run Docker containers again as requested to see if anything changes in regards to linter. 👍
For second screenshot you provided, I believe this message is coming from Search.tsx on lines 60-61.
I will review and see what I can tweak to get error message on same page as input vs separate page but is it possible to make a new ticket for that so we can get this ticket's work/current work merged in first? 👍

RivaD2 commented 2 years ago

@knjk04 can you review the above notes when you have time? I propose that we merge the work I have after I review the above with linter errors. I can then make a separate PR for the updates you want for the error message shown.

This way the PR is a bit more focused on the issue assigned/ easier to review. Is that cool with you? :)

knjk04 commented 2 years ago

Hi @RivaD2, sorry for the delay in getting back to you, I missed your previous message.

It seems we get the linter error on the main branch, but if you're able to replicate this, could we fix this so that we can test this out and so our dev environment doesn't break when we merge this in?

I propose that we merge the work I have after I review the above with linter errors. I can then make a separate PR for the updates you want for the error message shown.

Sure, that's a good plan!

Thanks a lot!

RivaD2 commented 2 years ago

@knjk04 I followed instructions in README.md for book-project:

ran docker-compose build
ran docker-compose --env-file .env up

I am getting this error:

My branch is now even with Main, and I believe I have fixed linter error. Once we can get app, I will take a closer look at linter/file to ensure my fix is good 👍

knjk04 commented 2 years ago

@RivaD2 What steps did you take that resulted in this error? Did you try logging in?

Could you also please paste the output into a GitHub gist?

RivaD2 commented 2 years ago

ran docker-compose build

ran docker-compose --env-file .env up

@knjk04 The steps I took were mentioned above ( I ran the two commands listed on readme.md for book-project to start the app). What login is required is to start the projects? The only thing I usually do, is when dev server is running, I navigate to localhost , at that point yes, I sign in to the project. Here though the dev server won't start.

Here is a link to my public gist:

https://gist.github.com/RivaD2/d2d74c0904feb830ce475be12f035c5d

RivaD2 commented 2 years ago

@knjk04 See updated gist:

https://gist.github.com/RivaD2/d2d74c0904feb830ce475be12f035c5d

it shows full output errors. I added in .env vars since they were missing after I pulled in all changes and got my branch even with main.

The last section in the gist shows errors after I added in .env vars. When I run docker-compose down I see that the book-project backend and book-project db are stopped, but nothing is mentioned about frontend...perhaps this is normal behavior, just thought I would mention that here).

RivaD2 commented 2 years ago

@knjk04, Per slack discussion, after you fixed Issue with executable flag in script, the frontend now is running.

Now when starting books-api, the books-api db is not running properly. I followed instructions in repo for starting books api.

After running commands in books-api directory, in Docker, it shows that the versions aren't compatible/database files are incompatible with server:

Do I need to update any version of PostgreSQL on my end, or is this from your end?

UPDATE: The questions here were discussed in team Slack, links to issue #973

stale[bot] commented 2 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

RivaD2 commented 2 years ago

@knjk04 I hope all is well!

It looks like there is a branch for fixing PostgreSQL issue, is that correct? I know we discussed this issue via Slack as well awhile back. I will be able to do my last task and merge this PR as soon as I can get container up and running 👍

stale[bot] commented 2 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Project-Books / book-project