Flask-AppBuilder icon indicating copy to clipboard operation
Flask-AppBuilder copied to clipboard

fix: primary keys need not to be integers, to serialize them is safer to use str() than int()

Open mapio opened this issue 2 years ago • 4 comments

This patch fixes #1805.

mapio avatar Feb 16 '22 07:02 mapio

Codecov Report

Merging #1806 (a4cb837) into master (c6fecdc) will not change coverage. The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #1806   +/-   ##
=======================================
  Coverage   78.29%   78.29%           
=======================================
  Files          72       72           
  Lines        8699     8699           
=======================================
  Hits         6811     6811           
  Misses       1888     1888           
Flag Coverage Δ
python 78.29% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
flask_appbuilder/views.py 66.11% <0.00%> (ø)

codecov[bot] avatar Feb 16 '22 07:02 codecov[bot]

Makes sense to me, lint is failing on CI

It actually seems that the fail on CI is due to codecov reporting that the patched lines are not covered by tests; it actually seems that all the lines in the modified file are not covered at all by any test.

I ask you to approve and merge the patch, since it will be difficult for me to add tests for all the views.

mapio avatar Mar 04 '22 09:03 mapio

Is there reason why this isn't merged yet?

mapio avatar Mar 10 '22 15:03 mapio

@mapio I think Daniel's style on merging in this repo is bit different, he will approve all finalised prs and just before cutting a release he will rebase on master and merge.

mayurnewase avatar Mar 12 '22 08:03 mayurnewase