[Phpmyadmin-devel] Grid editing

[Aris Feryanto]

> From: Marc Delisle <marc at infomarc.info>

> Aris Feryanto a écrit :
>>>  From: Marc Delisle <marc at infomarc.info>
>> 
>>>  Le 2011-07-31 13:15, Aris Feryanto a écrit :
>>> 
>>>>>>>   3. I hope that we'll get the OK from other 
> developers to 
>>>  remove
>>>>>>>   the row inline edit feature, because with the current 
> code,
>>>>>>> 
>>>>>>>   3.1 we have two features to do almost the same thing 
> 3.2
>>>>>>>   currently, clicking on inline edit then on a cell 
> activates the
>>>>>>>   grid editing!
>>>>>>   I hope so. Any objections?
>>>  Still waiting for input from other developers...
>> 
>>  It seems there is no objection about this. Instead, positive feedback from 
> other phpMyAdmin developers.
> 
> Yes. Can you remove the row inline edit feature from your tree?

Sure. I will work on this.

> 
>> 
>>>>>>>   4. With this feature, we have the same problems as 
> with inline 
>>>>>>>   edit, such as:
>>>>>>> 
>>>>>>>   4.2 (major) For tables with a unique key, if this key 
> is
>>>>>>>   edited, the Edit, Copy and Delete links no longer 
> contain a
>>>>>>>   valid where_clause parameter and the multi-rows 
> actions at the
>>>>>>>   bottom (with selected) no longer work.
>>>>>>> 
>>>>>>>   4.3 (major) For tables without a unique key, if any 
> column is 
>>>>>>>   edited, the Edit, Copy and Delete links no longer 
> contain a
>>>>>>>   valid where_clause parameter and the multi-rows 
> actions at the
>>>>>>>   bottom (with selected) no longer work.
>>>>>> 
>>>>>>   Two problems above were fixed.
>>>>>   It's a good progress but the fix is not complete.
>>>>> 
>>>>>   About remark 4.2, the Delete link still refers to the old 
> unique
>>>>>   key.
>>>>   It's weird, since this works in my test. Could you please 
> check this
>>>>   again?
>>>  The real problem is that the confirmation message shown before deletion
>>>  still contains the old key. Deletion itself works but I had stopped to
>>>  test after seeing the wrong confirmation message.
>> 
>>  Fixed
> 
> Confirmed.
> 
>> 
>>>>>   About multi-row actions, here is a scenario: - open 
> sakila.actor -
>>>>>   use the checkbox on two rows - in the "With 
> selected", click
>>>>>   Export - it works: you are seeing these keys in the generated
>>>>>   query
>>>>> 
>>>>>   - go back to browsing sakila.actor - grid edit one row, change 
> the
>>>>>   unique key and save - use the checkbox on two rows, including 
> the
>>>>>   one you just grid-edited - in the "With selected", 
> click 
>>>  Export -
>>>>>   problem: incorrect generated query for the grid-edited row
>>>>   Yap. It still generates wrong export. I'll fix this.
>>>> 
>>>>>   About remark 4.3, the Edit link still points to the old data 
> and
>>>>>   fails, in the case of a table lacking a unique key.
>>>>> 
>>>>   Same as the response for 4.2, this works on my test. Could you 
> check
>>>>   this again?
>>>  I confirm the problem, on a copy of the sakila.actor table where I have
>>>  removed all indexes. I think it's because the last_update column (a
>>>  timestamp) was updated via grid-editing but the URL still contains the
>>>  previous value.
>> 
>>  Fixed.
> 
> Problem 4.3 exists in your current tree. Tested with the Edit link on 
> the sakila.actor table by changing the actor_id.

In my test, grid-editing the actor_id in sakila.actor table and then click the Edit link direct me to correct page, where I can edit the row.
Btw, remark 4.3 is for tables without a unique key, while actor_id is primary key. Is this correct?

> 
>> 
>>>  5. Just found out another problem, when the configuration AjaxEnable is
>>>  set to false.
>>> 
>>>  Trying your version with latest commit
>>>  0d197603085c43cac543351795b3df6de4167b96 with Firefox 5 on 
> sakila.actor,
>>>  I cannot change the actor_id then click "Save edited data". 
> Look what 
>>>  is
>>>  generated:
>>> 
>>>  UPDATE `sakila`.`actor` SET = '4002' WHERE `actor2`.`actor_id` 
> =2
>>> 
>> 
>> 
>>  Regarding the AjaxEnable setting, should the grid editing works when 
> AjaxEnable is set to off?
> 
> No.

I will work on this.

> 
> 6. As suggested by Dieter and confirmed by Michal, please set this in 
> config.default.php:
> 
> $cfg['SaveCellsAtOnce'] = false;
> 
> Indeed it seems more prudent to auto-save, even it means more requests 
> in case of many changes.

Updated and pushed to my git.

--
Aris Feryanto