acikyazilimagi / deprem-yardim-frontend

release canditate: https://rc.afetharita.com/
https://afetharita.com/
Apache License 2.0
896 stars 283 forks source link

fix(#1205): deprem io response fix #1213

Closed absolutezero13 closed 1 year ago

absolutezero13 commented 1 year ago

Description

discord username: afetharita#0001

closes #issue #1205 #1210

Please describe your changes. Also describe your aim and content. Do not forget to list the dependencies required caused by those changes.

 Things to check before creating a PR

Creating PR rules

Changes

How were these changes tested?

Please describe the tests you did to test the changes you made. Please also specify your test configuration.

Test Configuration:

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
deprem-yardim-frontend ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 9:05AM (UTC)
yusufmeteyilmaz commented 1 year ago

LGTM!

yusufmeteyilmaz commented 1 year ago

I'm sorry, I was rash. There is a problem. Not every request returns the same response, I mean some of them contain insufficient or irrelevant information. I think we should use "full_text".

For example: id:1614534 Api: https://apigo.afetharita.com/feeds/1614534 Afet harita: https://afetharita.com/en?id=1614534 Screenshot: image Rc: https://rc.afetharita.com/en?id=1614534 Screenshot: image vercel preview: https://deprem-yardim-frontend-6xmb3tl96-afet.vercel.app/en?id=1614534 Screenshot: image Response: image image

absolutezero13 commented 1 year ago

@hackerone-yusuf I've changed it to full_text. Can you check?

yusufmeteyilmaz commented 1 year ago

Yes, it works as expected now, thanks. But I saw the email info on one of them, I guess this kind of personal information should be filtered again. Should I open a new issue for that?

absolutezero13 commented 1 year ago

Yes, it works as expected now, thanks. But I saw the email info on one of them, I guess this kind of personal information should be filtered again. Should I open a new issue for that?

Hm, I am not sure about this to be honest. But I don't think there should be another issue since this is not merged.

yusufmeteyilmaz commented 1 year ago

@absolutezero13 Alright then, thank you again.

usirin commented 1 year ago

... because node v14 doesn't support replaceAll.

i am confused, why are we making a change for node 14?

absolutezero13 commented 1 year ago

@usirin I'm not sure where but while setting up this project there was a spesific node version requirement that is around v14 or something. But if it's node v15 or above in production then it will be fine but even then there is no need to use replaceAll when there is regex.