remove http/s from orchid while searching for person

datacite / akita

DataCite Commons

https://commons.datacite.org

MIT License

6 stars 3 forks source link

remove http/s from orchid while searching for person #299

Closed wendelfabianchinsamy closed 10 months ago

wendelfabianchinsamy commented 11 months ago

Purpose

closes: https://github.com/datacite/datacite/issues/1786

Approach

Open Questions and Pre-Merge TODOs

Learning

Types of changes

[x] 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 change)

Reviewer, please remember our guidelines:

Be humble in the language and feedback you give, ask don't tell.
Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
Offer suggestions on how to improve code e.g. simplification or expanding clarity.
Ensure you give reasons for the changes you are proposing.

cypress[bot] commented 11 months ago

Passing run #961 ↗︎

Details:

Merge 3b345cc839ec41f05dca3c35034269ea1aba5028 into 9c9349377b7ed6207a2ac16e6db7...
Project: akita	Commit: `36dc3120af ℹ️`
Status: Passed	Duration: 02:46 💡
Started: Sep 27, 2023 1:26 PM	Ended: Sep 27, 2023 1:29 PM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

codycooperross commented 11 months ago

Nice fix! Noting that it might be good to handle search query string normalization on the lupo/GraphQL API side in addition to akita/Commons. I think this fix will work well for queries conducted in akita/Commons but won't address GraphQL queries conducted elsewhere.

wendelfabianchinsamy commented 11 months ago

Nice fix! Noting that it might be good to handle search query string normalization on the lupo/GraphQL API side in addition to akita/Commons. I think this fix will work well for queries conducted in akita/Commons but won't address GraphQL queries conducted elsewhere.

@codycooperross great point. @ashwinisukale and @kaysiz, would you agree then that it would be best to implement this fix on the lupo/Graphql end instead of here. If so we could probably nuke this pr.

kaysiz commented 11 months ago

@wendelfabianchinsamy I agree, lets nuke this PR in favor of the one on lupo