Skip to content

Feature: Search Issues using GitHub search API#523

Merged
ankushdharkar merged 9 commits into
RealDevSquad:developfrom
bharati-21:feature/144-search-issues
May 4, 2023
Merged

Feature: Search Issues using GitHub search API#523
ankushdharkar merged 9 commits into
RealDevSquad:developfrom
bharati-21:feature/144-search-issues

Conversation

@bharati-21
Copy link
Copy Markdown
Contributor

Feature: Search Issues using GitHub search API

This PR addresses the following issue to search for issues across RDS repositories

  • The following changes are part of this PR:
    • Users can search for specific issues by inputting query string in an input box. Submitting the query calls the GET /issues API, which in turn calls the appropriate GitHub API.
    • Test case to validate issues pages with API calls mocked using mock service worker.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 3, 2023

@bharati-21 is attempting to deploy a commit to the RDS-Team Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown

@vvaibhavdesai vvaibhavdesai left a comment

Choose a reason for hiding this comment

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

there are few component please address those

Comment thread src/pages/issues.tsx Outdated
Comment thread src/pages/issues.tsx
const [issueList, setIssueList] = useState<[]>([]);
const [searchText, setSearchText] = useState('');
const [isLoading, setIsLoading] = useState(true);
const [error, setError] = useState<null | any>(null);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

???

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can you please elaborate?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

why is the error state set to any ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When an error is caught, the error state is set to the error object - the type of which is unknown. Hence, have set to any | null.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

But I think the error object must have a defined structure else it would be kind of difficult to do error handling on frontend

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is difficult to do so because the error here is from the GitHub API - which is not feasible to define.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

have we checked this? I am sure GitHub might have something for a use case as well Link

Copy link
Copy Markdown
Contributor Author

@bharati-21 bharati-21 May 4, 2023

Choose a reason for hiding this comment

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

@vvaibhavdesai Yes, even in the API there are sample responses for error cases.

But while fetching issues from the backend, along with GitHub API, there are few calls made to the firestore data models as well. Predicting errors in this case is not exhaustive and missing out any of the error scenarios could lead to issues in the frontend.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bharati-21 Please mention the ticket for the future changes here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread src/pages/issues.tsx Outdated
Comment thread src/pages/issues.tsx
Comment thread src/pages/issues.tsx
Comment thread src/pages/issues.tsx Outdated
Comment thread src/pages/issues.tsx
Comment thread src/pages/issues.tsx
Comment thread src/pages/issues.tsx Outdated
Comment thread src/pages/issues.tsx Outdated
Comment thread __tests__/Unit/Pages/Issues.test.tsx
Comment thread __tests__/Unit/Pages/Issues.test.tsx
Comment thread src/pages/issues.tsx
Comment thread src/pages/issues.tsx
@bharati-21 bharati-21 requested a review from vvaibhavdesai May 4, 2023 06:27
Comment thread src/pages/issues.tsx
Comment thread __mocks__/db/issues.ts
Comment thread __tests__/Unit/Pages/Issues.test.tsx
Comment thread __tests__/Unit/Pages/Issues.test.tsx
@bharati-21 bharati-21 requested a review from bhtibrewal May 4, 2023 08:43
Copy link
Copy Markdown
Contributor

@bhtibrewal bhtibrewal left a comment

Choose a reason for hiding this comment

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

LGTM

@ankushdharkar ankushdharkar merged commit 0a1a601 into RealDevSquad:develop May 4, 2023
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.

4 participants