fix(admin): correct dict literal syntax in import_entries warning accumulation#490
fix(admin): correct dict literal syntax in import_entries warning accumulation#490ayushshukla1807 wants to merge 2 commits intohatnote:masterfrom
Conversation
lgelauff
left a comment
There was a problem hiding this comment.
The fix on line 370 is correct — {'import issues', msg} creates a Python set which can't be JSON serialized, causing a 500 error whenever add_entries_by_name() returns warnings. Changing it to {'import issues': msg} is the right fix.
However, the same bug exists one branch up, in the csv/gistcsv branch (line 351):
import_warnings.append(msg) # plain stringThe frontend processes warnings with Object.values(warning).pop(), which expects each entry to be an object. When passed a plain string, Object.values() returns the individual characters, so .pop() yields only the last character of the message instead of the full text.
Please change line 351 to:
import_warnings.append({'import issues': msg})to make it consistent with the other branches. Then this PR can be approved.
…rendering
The csv/gistcsv branch was appending a plain string to import_warnings.
The frontend processes warnings with Object.values(warning).pop(), which
expects each entry to be an object — passing a string yields individual
characters, so only the last character would be displayed.
Wraps the message in {'import issues': msg} to match all other branches.
lgelauff
left a comment
There was a problem hiding this comment.
I've pushed one additional fix to this branch: the csv/gistcsv branch had the same underlying issue — it was appending a plain string to import_warnings instead of a dict.
The frontend processes warnings with Object.values(warning).pop(). On a plain string, Object.values() returns individual characters, so only the last character of the message would be displayed to the coordinator.
Changed line 351 from:
import_warnings.append(msg)to:
import_warnings.append({'import issues': msg})This makes all branches consistent. Happy to approve once you've had a chance to review the addition.
|
Hey @lgelauff, really good catch on the csv branch. I honestly completely missed that the exact same bug was sitting over there. I just went through your commit, and wrapping the payload in the dict is definitely the right move so the frontend doesn't accidentally strip out the last character of the raw string. I really appreciate you taking the time to write and push that fix straight to the branch for me. Everything looks perfect on my end, so feel free to approve and merge whenever you're ready. Thanks again! |
Fixes a data drop in
admin_endpoints.pywhere import warnings were being lost instead of passed to the frontend.In
import_entries, the CSV/uploaded files branch was building a Python set forwarning_messagesinstead of a dict like the other branches. When it tried to JSON serialize this for the response, the backend threw an error. I just updated it to use standard dict syntax{{}}so the warnings surface correctly.