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

Fix[BB-513] : added missing entity option for every search and prefilled the name #722

Merged
merged 6 commits into from
Jan 25, 2022

Conversation

tr1ten
Copy link
Collaborator

@tr1ten tr1ten commented Nov 30, 2021

Problem

BB-513 : Improve "Are we missing an entry?" section.

Solution

Added missing entity option in every search result and prefilled the new entity with searched name using query parameters.

Areas of Impact

  1. src/client/components/pages/parts/call-to-action.js
  2. src/client/components/pages/parts/search-results.js
  3. src/client/components/pages/search.tsx
  4. src/server/routes/entity/author.js
  5. src/server/routes/entity/edition-group.js
  6. src/server/routes/entity/edition.js
  7. src/server/routes/entity/publisher.js
  8. src/server/routes/entity/work.js

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Nice work so far.
I have some suggestions to improve a few things but generally you're going in the right direction, nicely done :)

src/client/components/pages/parts/search-results.js Outdated Show resolved Hide resolved
src/server/routes/entity/author.js Outdated Show resolved Hide resolved
src/client/components/pages/parts/call-to-action.js Outdated Show resolved Hide resolved
Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Last issue I found while testing, other than that it looks great, thanks !

src/client/components/pages/search.tsx Outdated Show resolved Hide resolved
tr1ten and others added 3 commits December 22, 2021 09:14
Re-applying modification after merging commit 9e7fc2e (the code in question has moved from search-results.js to search.tsx page)
@MonkeyDo
Copy link
Contributor

I took the liberty to fix a merge conflict, since it was not super trivial (nothing complicated but required knowing what moved where).

@MonkeyDo MonkeyDo merged commit 2358867 into metabrainz:master Jan 25, 2022
@tr1ten tr1ten deleted the fix#BB-513 branch January 27, 2022 03:31
MonkeyDo added a commit that referenced this pull request Feb 2, 2022
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.

2 participants