Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Designer adding all available tables to a designer page after adding a new relationship. #15720

Closed
NKarasek opened this issue Dec 29, 2019 · 10 comments
Assignees
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Milestone

Comments

@NKarasek
Copy link

Problem (Large):

Designer adding all available tables to a designer page after adding a new relationship.

Steps to replicate:

a.  build a new ER diagram and save it with a new name,
b.  reload the diagram,
c.  select "Show/Hide tables list" from (default) left-side menu,
d.  click any checked table name from list of tables shown in right-side list to remove it.
e.  NOTE that the "modified" indicator (an asterisk) is not displayed in Diagram Name (expected),
    nor is `table_coords updated (not expected).
f.  save the page - `table_coords IS updated (expected).
g.  reload the page - requested table in step d. is removed.
h.  select "Create Relationship" from (default) left-side menu and build a new relationship.
i.  NOTE that the "modified" indicator (an asterisk) is not displayed in Diagram Name (expected),
    nor is `table_coords updated (not expected).
j.  save the page - `table_coords` IS updated (expected) but ALL available tables are now 
    associated with the currentpage.
k.  The screen display is NOT updated, however, but ALL tables are checked after selecting 
    "Show/Hide tables list".
l.  select "Reload" from (default) left-side menu displays the current page with ALL tables
   displayed.

Expected result:

During step i. above, the "modified" indicator should be displayed.
During step j. above, `table_coords` should NOT be updated unless a table has been added or
removed from the display.

Problem (Small):

Designer not changing the tooltip 'title' of the checkbox selecting tables to show or hide
from list of tables shown when selecting "Show/Hide tables list" from (default) left-side menu.

Steps to replicate:

a.  display any designer page.
b.  select "Show/Hide tables list" from (default) left-side menu,
c.  hover the mouse cursor over any checked table show/hide checkbox.  Tooltip "Hide" is 
    displayed.
d.  click any of the checked table checkboxes to remove it from the displayed page.
e.  hover the mouse cursor over checkbox clicked in step d (now NOT checked).  Tooltip "Hide" is
     STILL displayed.

Expected result:

The tooltip shown when hovering over the "Show/Hide table name" checkbox should reflect the 
action that  will occur if the checkbox is clicked to change state.

Server configuration

  • Apache/2.4.41 (Win64) OpenSSL/1.1.1c PHP/7.3.10

  • Windows 10

  • Apache/2.4.41 (Win64)

  • OpenSSL/1.1.1c

  • Database version: libmysql - mysqlnd 5.0.12-dev (MariaDB)

  • PHP version: PHP/7.3.10

  • phpMyAdmin version: 5.0.0 (also in 4.9.2 - likely 4.9.3)

Client configuration

  • Browser: 79.0.3945.88 (Official Build) (64-bit) (cohort: Stable)
  • Operating system: Windows 10 OS Version 1903 (Build 18362.535)

Additional context

JS V8 7.9.317.32

@williamdes williamdes added the Bug A problem or regression with an existing feature label Dec 29, 2019
@williamdes williamdes added this to Needs triage in issues via automation Dec 29, 2019
@williamdes williamdes added this to To Do in Designer Dec 29, 2019
@williamdes williamdes moved this from Needs triage to to be fixed soon in issues Dec 29, 2019
@williamdes
Copy link
Member

Hi @NKarasek
Thank you for the detailed report

@NKarasek
Copy link
Author

You are very welcome. After further testing it became obvious the problem is more general. It seems that ANY page save-action to ANY page adds all available tables to table_coords for that page. If all that is done is to add a relationship, table_coords is NOT modified. Moving a table on the screen (causing the "modified" indicator to display), then saving the page is the trigger.

@yashrajbothra
Copy link
Contributor

yashrajbothra commented Dec 29, 2019

Hey @williamdes , can i work on this issue?

@williamdes
Copy link
Member

@yashrajbothra Yes, please use QA_5_0 as base
Your contribution will be very helpful and I will review it quickly since I have spent some time with the designer's codebase

@NKarasek
Copy link
Author

NKarasek commented Dec 29, 2019

@yashrajbothra - glad to see you will be on the task. Observation: - after cleaning up extra tables on a diagram - and saving - all is good. Extra tables are removed from 'table_coords'. After clicking "Show/Hide tables list", ALL tables in the list are selected. I would expect that ONLY those tables that are currently on-screen would be selected. It is possible that the array of available tables is not getting initialized correctly (which should be based on whatever tables are currently selected for the page.) If all tables are still selected, even after making NO changes to the current page and then saving the page, it is possible that the save routine is looking at the incorrectly initialized tables array and deducing that ALL tables that are clicked (which would be ALL tables after displaying the list) should be added to the page. I've looked through the code but am too sleepy atm to locate the appropriate lines. Hope this is on the right track and doesn't distract you from the correct solution :-) BTW, this behaviour is also present (at least on my workstation) in v4.9.1.

@yashrajbothra
Copy link
Contributor

yashrajbothra commented Dec 30, 2019

Hey @williamdes @NKarasek , Thanks for your explaination.I got the whole issue except the term table coordinates.
what is 'table_coords' ? how to see table coords and check wheather its updating or not?

@NKarasek
Copy link
Author

Sorry - PHPMyAdmin (PHPMA) uses the table 'phpmyadmin.PMA__table_coords' to save the current x,y position of each table in each Designer Page. As the participating tables are dragged within the page (moved to new locations), PHPMA saves the current x,y coordinates (coords) as the tables are dropped (mouse button released). PMAMA uses 'PMA__table_coords' to determine which tables are selected for display on the page. The problem I have reported is that as a page is saved AFTER MOVING A TABLE, PMAMA adds ALL tables in the database (those that aren't already on the current page) to table PMA__table_coords using the default x,y positions of each table. I'm guessing that the table name is used to hash both x and y positions and the current size of the user's monitor, as the default positions are the same each time they are added. Check into why the list of tables displayed after clicking "Show/Hide tables list" displays ALL tables with their "Show" checkbox checked. This list should ONLY show the tables that are already on the page as checked (for display).

@yashrajbothra
Copy link
Contributor

I understood the whole issue now and thanks @NKarasek for helping me out.I have started working on the issue.

yashrajbothra added a commit to yashrajbothra/phpmyadmin that referenced this issue Dec 30, 2019
yashrajbothra added a commit to yashrajbothra/phpmyadmin that referenced this issue Dec 31, 2019
yashrajbothra added a commit to yashrajbothra/phpmyadmin that referenced this issue Dec 31, 2019
Signed-off-by: Yash Bothra <yashrajbothra786@gmail.com>
@williamdes williamdes added the has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete label Jan 1, 2020
williamdes pushed a commit to yashrajbothra/phpmyadmin that referenced this issue Jan 1, 2020
Signed-off-by: Yash Bothra <yashrajbothra786@gmail.com>
@NKarasek
Copy link
Author

NKarasek commented Jan 9, 2020

Thanks for v501 and v494 but sorry to see that the work done for 15720 didn't make it into the new releases. Designer folks simply can't use designer until these small changes are made. :-(

@williamdes
Copy link
Member

Hi @NKarasek
The security fix rushed an upload and I could not merge all the fixes I had planned.
I will assign this one for next milestone

Thank you

@williamdes williamdes added this to the 5.0.2 milestone Jan 9, 2020
@williamdes williamdes self-assigned this Jan 15, 2020
williamdes added a commit that referenced this issue Jan 16, 2020
…esigner page after adding a new relationship.

Pull-request: #15727
Fixes: #15720
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Jan 16, 2020
Signed-off-by: William Desportes <williamdes@wdes.fr>
issues automation moved this from to be fixed soon to Closed Jan 16, 2020
@williamdes williamdes moved this from To Do to Done in Designer May 3, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Projects
issues
  
Closed
Development

No branches or pull requests

3 participants