Skip to content

Fix field SQL identifier render for queries /wo table alias#784

Closed
DarkSide666 wants to merge 7 commits intodevelopfrom
feature/test-ref
Closed

Fix field SQL identifier render for queries /wo table alias#784
DarkSide666 wants to merge 7 commits intodevelopfrom
feature/test-ref

Conversation

@DarkSide666
Copy link
Copy Markdown
Member

@DarkSide666 DarkSide666 commented Nov 19, 2020 •
edited by mvorisek
Loading

check if needed once #938 is fixed, this PR changes does not help

steps to reproduce:

  1. open http://localhost/.../atk4/login/demos/admin-roles.php
  2. click:
    image
  3. add "name" field and click save:
    image
  4. error like this is displayed:
    image

@DarkSide666 DarkSide666 marked this pull request as draft November 19, 2020 22:09
@georgehristov
Copy link
Copy Markdown
Collaborator

georgehristov commented Dec 8, 2020 •
edited
Loading

The issue is on this line:

data/src/Reference/HasOneSql.php

Line 191 in 7a39b62

$theirModel = $ourModel->refLink($this->link);

Model::refLink is used which adds aliases but in DSQL the operations apart from SELECT use template [table_noalias]

@DarkSide666
Copy link
Copy Markdown
Member Author

DarkSide666 commented Dec 9, 2020

Maybe we should have a way to tell refLink to not add aliases in some specific cases?

@mvorisek mvorisek mentioned this pull request Oct 22, 2021
Closed
@mvorisek mvorisek removed the bug label Nov 10, 2021
@mvorisek mvorisek closed this Nov 10, 2021
@mvorisek mvorisek deleted the feature/test-ref branch November 10, 2021 14:18
@mvorisek mvorisek restored the feature/test-ref branch November 19, 2021 12:42
@mvorisek mvorisek reopened this Nov 19, 2021
@mvorisek mvorisek added the bug label Nov 19, 2021
@mvorisek mvorisek mentioned this pull request Nov 19, 2021
Closed
@mvorisek mvorisek force-pushed the feature/test-ref branch 2 times, most recently from 95013d5 to b4fa410 Compare November 23, 2021 00:43
mvorisek
mvorisek requested changes Nov 23, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mvorisek mvorisek left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not convinced to fixing this issue for Field and for condition only as proposed by @samsouthardjr fix in #909. However the fix fixes the failing test.

@mvorisek mvorisek changed the title failing test case - reason bad implementation of withTitle + table_alias Fix field SQL identifier render for queries /wo table alias Nov 23, 2021
@mvorisek mvorisek force-pushed the feature/test-ref branch 2 times, most recently from 869cd7c to e23b926 Compare November 23, 2021 13:25
@mvorisek
Copy link
Copy Markdown
Member

mvorisek commented Nov 28, 2021

closing in favor of proper fix in #938 incl. test

@mvorisek mvorisek closed this Nov 28, 2021
@mvorisek mvorisek deleted the feature/test-ref branch November 28, 2021 13:31
@mvorisek mvorisek mentioned this pull request Nov 28, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mvorisek mvorisek mvorisek requested changes

Assignees

No one assigned

Labels

bug waitingForAnotherPR

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DarkSide666 @georgehristov @mvorisek