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

feat(#1316): Implemented search bar on org home page #17

Closed
wants to merge 39 commits into from

Conversation

AbdulWahab3181
Copy link
Contributor

@AbdulWahab3181 AbdulWahab3181 commented Jan 24, 2024

Describe your changes

video1235284858.mp4

Unit Test

image

Issue ticket number and link

Closes #190

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have tested on Chrome and Firefox
  • I have tested on a mobile device
  • I have provided a screenshot or recording of changes in my PR if there were updates to the frontend

@AbdulWahab3181
Copy link
Contributor Author

@ecurrencyhodler Could you please have a look?

@ecurrencyhodler
Copy link
Contributor

Video looks good. Sorry one more thing. We have reverted our backend api calls from api.people-test->people.test. So we no longer will require the "api" in front of the call.

Can you push another commit? I think that'll trigger our github actions workflows for testing. Also in the future, you can run the tests locally as well using: yarn run.

@AbdulWahab3181
Copy link
Contributor Author

Video looks good. Sorry one more thing. We have reverted our backend api calls from api.people-test->people.test. So we no longer will require the "api" in front of the call.

Can you push another commit? I think that'll trigger our github actions workflows for testing. Also in the future, you can run the tests locally as well using: yarn run.

Sure. Let me push another commit.

@ecurrencyhodler
Copy link
Contributor

Okay let's get a code review. Gj!

elraphty and others added 9 commits January 25, 2024 11:42
…-buttons

Implement Dynamic Display of Organization Website and GitHub Buttons in Org Home Page
Update the github readme for the frontend repo of sphinx-tribes
Fix(Bounty-Page): Enhanced Navigation for Direct Access to Bounty Pages
…person-page

Fix/1408 people search on person page
implement the status filter on the org homepage
@elraphty
Copy link
Collaborator

@AbdulWahab3181 Please fix merge conflicts.

@AbdulWahab3181
Copy link
Contributor Author

@AbdulWahab3181 Please fix merge conflicts.

@elraphty Resolved

onChange={(e: any) => {
ui.setSearchText(e);
}}
onKeyUp={(e: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try to avoid Inline functions, create the search function outside the component, and call it in the onKeyUp event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

@ecurrencyhodler
Copy link
Contributor

@elraphty this is from @AbdulWahab3181. org home page api call isn't working.

"When I make a request to the https://people-test.sphinx.chat/organizations/bounties/cmkln4tm098m49vhlt80?limit=10&sortBy=created&search=test&page=1&resetPage=true endpoint with the search parameter set to 'test,' I receive a null response. However, it works correctly with the 'gobounties/all' endpoint, as demonstrated here: https://people-test.sphinx.chat/gobounties/all?limit=10&sortBy=created&search=test&page=1&resetPage=true&Open=true&Assigned=false&Paid=false&org_uuid=cmkln4tm098m49vhlt80."

@AbdulWahab3181
Copy link
Contributor Author

@elraphty this is from @AbdulWahab3181. org home page api call isn't working.

"When I make a request to the https://people-test.sphinx.chat/organizations/bounties/cmkln4tm098m49vhlt80?limit=10&sortBy=created&search=test&page=1&resetPage=true endpoint with the search parameter set to 'test,' I receive a null response. However, it works correctly with the 'gobounties/all' endpoint, as demonstrated here: https://people-test.sphinx.chat/gobounties/all?limit=10&sortBy=created&search=test&page=1&resetPage=true&Open=true&Assigned=false&Paid=false&org_uuid=cmkln4tm098m49vhlt80."

I have created this pull request PR stakwork/sphinx-tribes#1460 to address the issue. Kindly review it and consider merging. Additionally, I've submitted another pull request PR #33 in the frontend repository to handle this issue. This new pull request utilizes the "organizations/bounties" endpoint for search, aligning with the approach used by other components such as status.

Considering the changes introduced in these new pull requests, I think we should have to close this PR as it is no longer necessary. The proposed modifications in the latest pull requests has adequately address the issue.

@ecurrencyhodler
Copy link
Contributor

Hey Abdul. Raph is going to take a look at both of those PR's and leave some comments for you.

@AbdulWahab3181
Copy link
Contributor Author

Hi Ecurrencyhodler, okay, I am waiting.

@elraphty
Copy link
Collaborator

@AbdulWahab3181 The Org bounty search is not working, I tested with the test server backend server and my local master branch

Test Server
https://www.loom.com/share/b376da26edb14574a2ac4ac648a6d3b3

Local Server
https://www.loom.com/share/91506e1e8b7a4441b58f52b8e0c22843

Fix it

@AbdulWahab3181
Copy link
Contributor Author

@AbdulWahab3181 The Org bounty search is not working, I tested with the test server backend server and my local master branch

Test Server https://www.loom.com/share/b376da26edb14574a2ac4ac648a6d3b3

Local Server https://www.loom.com/share/91506e1e8b7a4441b58f52b8e0c22843

Fix it

@elraphty Changes have been incorporated into #33 pull request. Please try it out and ensure that your local server or test server also reflect these changes from stakwork/sphinx-tribes#1460 and stakwork/sphinx-tribes#1467. I intend to close this pull request as it is no longer in use.

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.

Implement search bar on org home page
9 participants