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

Update namespace. Add table_name config to all models #39

Merged
merged 3 commits into from
Nov 16, 2021

Conversation

chrispenny
Copy link
Collaborator

@chrispenny chrispenny commented Nov 11, 2021

Prefixed each with Link_ to make it easier to find all relevant tables, but also because FileLink is already used by framework.

Copy link
Collaborator

@mfendeksilverstripe mfendeksilverstripe left a comment

Choose a reason for hiding this comment

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

I think we should go with a less generic table names to prevent potential naming conflicts. I can definitely see table called Link having a conflict especially if a project is migrating from another module. I added LinkFieldLink prefix as the name of this module is LinkField.

src/Models/EmailLink.php Outdated Show resolved Hide resolved
src/Models/ExternalLink.php Outdated Show resolved Hide resolved
src/Models/FileLink.php Outdated Show resolved Hide resolved
src/Models/Link.php Outdated Show resolved Hide resolved
src/Models/SiteTreeLink.php Outdated Show resolved Hide resolved
@chrispenny chrispenny changed the title Add table_name config to all models Update namespace. Add table_name config to all models Nov 15, 2021
Copy link
Collaborator

@mfendeksilverstripe mfendeksilverstripe left a comment

Choose a reason for hiding this comment

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

@chrispenny The changes look good but it seems some of the functionality is broken because of this change. This is my test setup (TNZ core after updating namespace references):

  • Block page with a TextBlock / ButtonLink
  • I switcher the block to NOT be inline editable
  • navigated to the block edit form

Expected:

Screen Shot 2021-11-16 at 8 45 37 AM

Actual:

Screen Shot 2021-11-16 at 8 42 20 AM

Maybe there are some class references in the React bundle as well?

@mfendeksilverstripe
Copy link
Collaborator

@chrispenny Can you please check if you are able to save and retain link related changes? I can see that the UI now works but none of the changes I make are retained. Seems to be an issue on both inline edit and normal edit form cases.

Create a link

Screen Shot 2021-11-16 at 11 41 11 AM

Hit save

Screen Shot 2021-11-16 at 11 41 20 AM

Link doesn't show up even after page reload.

@chrispenny
Copy link
Collaborator Author

Hi @mfendeksilverstripe,

These seem to be working ok for me.

Inline editor (text block):
Screen Shot 2021-11-16 at 1 29 23 PM

Standard editor (cta block), straight after a publish:
Screen Shot 2021-11-16 at 1 30 49 PM

Standard editor (cta block), after a refresh:
Screen Shot 2021-11-16 at 1 31 01 PM

I don't believe I have done anything special other than a dev/build and appropriate flushes.

Copy link
Collaborator

@mfendeksilverstripe mfendeksilverstripe left a comment

Choose a reason for hiding this comment

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

Tested on local and it's working fine 👍
thanks for contribution :)

@mfendeksilverstripe
Copy link
Collaborator

@chrispenny Feel free to merge.

@chrispenny chrispenny merged commit 1068b78 into 1 Nov 16, 2021
@chrispenny chrispenny deleted the bugfix/define-table-names branch November 16, 2021 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants