helapkg / hela

:icecream: Powerful software development experience and management. Enhancing @tc39 JS, @denoland and @nodejs, because we need a bit of magic. :sparkles: You can think of it as Cargo for the JavaScript ecosystem.
Mozilla Public License 2.0
331 stars 41 forks source link

Fix ampersand #81

Closed manuscriptmastr closed 7 years ago

manuscriptmastr commented 7 years ago

So looks like that bit of code that rebuilt a query string then reparsed was causing issues. I pulled the fields straight from IncomingForm, then handled the edge cases of (1) a field that already has a value and (2) a field that is already an array of values. To make sure file-related fields took precedence, I separated regular fields from file fields, then Object.assign()ed them to the fields property when finished.

Tests all pass and works perfectly on my site now.

Closes https://github.com/tunnckoCore/koa-better-body/issues/80

tunnckoCore commented 7 years ago

Need tests for lines 58 and 211. Please add or tweak the tests :)

Thanks for the PR in any way.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 52fee8f945c127ec6f20709f6de37770d6558740 on manuscriptmastr:fix-ampersand into 61d2beed80037773cfe55cd23e6f75932e5f3345 on tunnckoCore:master.

manuscriptmastr commented 7 years ago

Done. Added a test for a field with three values and removed the unused utils.parseQs function.

tunnckoCore commented 7 years ago

Yea, it's better now :) Services are green too. Thanks.

manuscriptmastr commented 7 years ago

👍 thanks!