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

3007: Fix feeback rating for no search results as negative #3036

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lunars97
Copy link
Contributor

@lunars97 lunars97 commented Jan 10, 2025

Short description

The feedback for search requests with no results is currently being sent as neutral. The implemented solution ensures that feedback will automatically be marked as negative when no results are found.

  • Set rating to negative if there are no search results
  • Adjust tests

Side effects

  • none

Resolved issues

Fixes: #3007


@lunars97 lunars97 force-pushed the 3007-feedback-for-no-search-results-should-be-negative branch 2 times, most recently from e4e5289 to 6d6e949 Compare January 12, 2025 18:50
@lunars97 lunars97 force-pushed the 3007-feedback-for-no-search-results-should-be-negative branch from 6d6e949 to cfdbd3e Compare January 12, 2025 19:17
@@ -11,7 +11,7 @@ type FeedbackModalContainerProps = {
}

const FeedbackModalContainer = ({ route }: FeedbackModalContainerProps): ReactElement => (
<FeedbackContainer {...route.params} />
<FeedbackContainer {...route.params} noResults={false} />
Copy link
Contributor

Choose a reason for hiding this comment

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

noResults is optional you can just add noResults = false at the FeedbackContainer props and remove this..

Suggested change
<FeedbackContainer {...route.params} noResults={false} />
<FeedbackContainer {...route.params} />

@@ -33,7 +41,10 @@ const FeedbackContainer = ({ query, language, routeType, cityCode, slug }: Feedb

useEffect(() => {
setSearchTerm(query)
}, [query])
if (noResults === true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (noResults === true) {
if (noResults) {

it's already a boolean

@@ -28,11 +28,12 @@ describe('FeedbackContainer', () => {

const city = 'augsburg'
const language = 'de'
const noResults = false
Copy link
Contributor

Choose a reason for hiding this comment

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

As long you add the default value false for noResults this is not needed

Suggested change
const noResults = false


it('should send feedback request with rating and no other inputs on submit', async () => {
const { getByText, findByText } = render(
<NavigationContainer>
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} />
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} noResults={noResults} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} noResults={noResults} />
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} />

@@ -70,7 +71,7 @@ describe('FeedbackContainer', () => {
const contactMail = '[email protected]'
const { getByText, findByText, getAllByDisplayValue } = render(
<NavigationContainer>
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} />
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} noResults={noResults} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} noResults={noResults} />
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} />

@@ -106,7 +107,7 @@ describe('FeedbackContainer', () => {
it('should disable send feedback button if rating button is clicked twice', async () => {
const { getByText, findByText } = render(
<NavigationContainer>
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} />
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} noResults={noResults} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} noResults={noResults} />
<FeedbackContainer routeType={CATEGORIES_ROUTE} language={language} cityCode={city} />

@@ -22,9 +22,17 @@ export type FeedbackContainerProps = {
cityCode: string
query?: string
slug?: string
noResults: boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
noResults: boolean
noResults?: boolean

I think there is noResults only when I search and 0 results so it's false most of the time until we set it to true.

routeType,
cityCode,
slug,
noResults,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
noResults,
noResults = false,

language={language}
cityCode={city}
query={query}
noResults={noResults}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
noResults={noResults}

language={language}
cityCode={city}
query={query}
noResults={noResults}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
noResults={noResults}

language={language}
cityCode={city}
query='query'
noResults={noResults}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
noResults={noResults}

@@ -56,7 +56,7 @@ export const FeedbackContainer = ({
query,
slug,
searchTerm,
isPositiveRating,
isPositiveRating: noResults === true ? false : isPositiveRating,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
isPositiveRating: noResults === true ? false : isPositiveRating,
isPositiveRating: noResults ? false : isPositiveRating,

@bahaaTuffaha
Copy link
Contributor

Sorry about these comment should be in one review instead of 10 😅

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.

Feedback for no search results should be negative
2 participants