Fixes #2014: Adds search service to the front-end #2140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
humphd
requested changes
Apr 14, 2021
There was a problem hiding this comment.
I updated Vercel to add the Search URL env var:
14:44:10.365 | Running "npm run build"
-- | --
14:44:10.659 | > @senecacdot/[email protected] build /vercel/workpath0
14:44:10.659 | > next build && next export
14:44:11.683 | info - Using webpack 5. Reason: future.webpack5 option enabled https://nextjs.org/docs/messages/webpack5
14:44:11.881 | Trying to load missing ENV variables from /.env
14:44:11.881 | Trying to load missing ENV variables from /config/env.development
14:44:11.881 | Using NEXT_PUBLIC_WEB_URL=https://telescope-c64dfl18s-humphd.vercel.app
14:44:11.881 | Using NEXT_PUBLIC_API_URL=https://dev.telescope.cdot.systems
14:44:11.881 | Using NEXT_PUBLIC_IMAGE_URL=https://dev.api.telescope.cdot.systems/v1/image
14:44:11.881 | Using NEXT_PUBLIC_POSTS_URL=https://dev.api.telescope.cdot.systems/v1/posts
14:44:11.881 | Using NEXT_PUBLIC_AUTH_URL=https://dev.api.telescope.cdot.systems/v1/auth
14:44:11.881 | Using NEXT_PUBLIC_SEARCH_URL=https://dev.api.telescope.cdot.systems/v1/search
But it's getting a 404 from https://dev.api.telescope.cdot.systems/v1/search/query?text=react&filter=post&page=0. What am I missing? Is it the extra /query? in that URL?
docker/production.yml
Outdated
| @@ -141,6 +143,7 @@ services: | |||
| environment: | |||
| - ELASTIC_URL=http://elasticsearch | |||
| - ELASTIC_PORT=9200 | |||
| - SEARCH_PORT | |||
There was a problem hiding this comment.
This shouldn't be necessary, we do it in docker-compose.yml.
Think it's this piece that @manekenpix mentioned when fixing the search url awhile back
|
manekenpix
reviewed
Apr 15, 2021
| @@ -54,7 +54,9 @@ const SearchResults = () => { | |||
| const { textParam, filter } = useSearchValue(); | |||
|
|
|||
| const prepareUrl = (index: number) => | |||
| `${telescopeUrl}/query?text=${encodeURIComponent(textParam)}&filter=${filter}&page=${index}`; | |||
| `${searchServiceUrl}/query?text=${encodeURIComponent( | |||
There was a problem hiding this comment.
it works without query in the URL
Suggested change
| `${searchServiceUrl}/query?text=${encodeURIComponent( | |
| `${searchServiceUrl}?text=${encodeURIComponent( |
There was a problem hiding this comment.
If you want to keep using query, this line should be:
service.router.use('/query', query);There was a problem hiding this comment.
We don't want /query any more, please remove.
humphd
requested changes
Apr 15, 2021
humphd
approved these changes
Apr 15, 2021
manekenpix
approved these changes
Apr 15, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue This PR Addresses
This PR closes #2014
Type of Change
Description
Now that the search service is functional and merged, we can finally work towards moving it to the front-end. This PR addresses that by adding the search service related arguments to the Dockerfile, the production.yml file, the next.config.js file, the config.ts file, and finally the relevant components that make requests to this endpoint. One bug in the search service is preventing this from working and so I'll need to address that before this PR is ready for review.
Checklist