Re: [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch][RM_2400]: Columns with defaults set to NULL when removing contents after pasting in the edit grid
Date: 2017-05-18 05:22:52
Message-ID: CAM5-9D_qkFCkGBS_PLF34R2838gsk5dxOcfzbYwcK4m=R8jJTw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

Please review the updated patch.

On Wed, May 17, 2017 at 8:46 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Wed, May 17, 2017 at 3:08 PM, Surinder Kumar <
> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> *Implementation:*
>>
>> 1) Took a flag 'is_row_copied' for each copied row:
>>
>> - to distinguish it from add/update row.
>> - to write code specific to copied row only as it requires different
>> logic than add row.
>>
>> 2) After pasting an existing row:
>>
>> - If a user clear the cell value, it must set cell to [default] value if
>> default value is explicitly given for column while creating table otherwise
>> [null].
>>
>> - Again, if a user entered a value in same cell, then removes that
>> value, set it to [null] (the same behaviour is for add row).
>>
>> 3) Took a 2-dimensional array "grid.copied_rows" to keep track the
>> changes of each row's cell so that changes made to each cell are
>> independent and removed once data is saved.
>>
>> 4) On pasting a row, the cell must be highlighted with light green
>> colour, so triggers an addNewRow event. Now copied row will add to grid
>> instead of re-rendering all rows again.
>>
>> 5) Moved the common logic into functions.
>>
>> This patch pass the feature test cases and jasmine test case.
>> Also I will add the test cases for copy row and send an updated patch.
>>
>> Please find attached patch and review.
>>
>
> Looks good. I saw the following issues:
>
> - The "new" row is not available once I've created the first new row, or
> pasted some.
>
​Fixed.

>
> - Rows are pasted in reverse order.
>
​Fixed.

>
> - If I copy/paste a new row that has yet to be saved, none of the values
> are actually copied.
>
​Fixed.​

>
> - Attempting to save a row that contains all null/default values results
> in an invalid query:
>
> INSERT INTO public.defaults (
> ) VALUES (
> );
>
> I think the only answer here is to explicitly insert NULL into any columns
> *without* a default value.
>
​I could not reproduce, However #3 might have fixed it too.​

Apart from this, I noticed epicRandomString(...) function doesn't generate
unique number always, due to this save and delete rows was affected. Not
all rows saved or deleted. The new function always returns a unique random
number.
Fixed.

>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
RM_2400_v1.patch application/octet-stream 21.4 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2017-05-18 06:13:57 Re: [pgAdmin4] [PATCH] History Tab rewrite in React
Previous Message Matthew Kleiman 2017-05-17 15:45:45 Re: [patch] upgrade slickgrid