NEW Allowed link types #137

sabina-talipova · 2023-12-07T00:21:10Z

Description

Added a new trait AllowedLinkClassesTrait that adds methods to set available link types or exclude them.
The Registry::class has been removed.
Files and methods for working with GraphQL have also been removed
Added a service class to generate available existing Link classes

Parent issue

Controlling link type per field #15

maxime-rainville

That's a good solid first pass. I tested it locally and it works great.

There wasn't an explicit AC for this, but I think logically we should have an exception thrown if you try to save a link of a type that is disallowed for the field. e.g. Your LinkField is configured to only allow EmailLink, but you do a manual AJAX query to save a SiteTreeLink into it.

README.md

src/Services/LinkTypeService.php

src/Traits/AllowedLinkClassesTrait.php

maxime-rainville · 2023-12-11T11:25:50Z

src/Traits/AllowedLinkClassesTrait.php

+     */
+    public function setDisabledTypes(array $types): static
+    {
+        if (is_array($types) || !empty($types)) {


Suggested change

if (is_array($types) || !empty($types)) {

if (!empty($types)) {

src/Traits/AllowedLinkClassesTrait.php

maxime-rainville

My biggest concern is that this won't work with Elemental.

I left a link to how to react props so it works with Elemental. If it doesn't make sense let me know, and we can go through it together.

README.md

src/Form/Traits/AllowedLinkClassesTrait.php

tests/php/Traits/AllowedLinkClassesTraitTest.php

src/Form/Traits/AllowedLinkClassesTrait.php

src/Models/Link.php

src/Services/LinkTypeService.php

src/Traits/AllowedLinkClassesTrait.php

sabina-talipova · 2023-12-17T23:45:05Z

#137 (comment)
Updated. Now user gets InvalidArgumentException if he's trying to pass empty array or non-Link type classes in setAllowedTypes.

maxime-rainville

It's almost there. Just a few ambiguities left to address.

maxime-rainville · 2023-12-18T04:22:44Z

src/Models/Link.php

+     */
+    public function getShortCode(): string
+    {
+        return strtolower(str_replace([' ', 'Link'], '', ClassInfo::shortName($this))) ?? '';


This is a minor tweak.

ClassInfo::shortName won't return spaces, so you don't need to remove spaces.

It would a bit safer to only remove the Link part if it's at the end of the class name. So using rtrim would be a bit safer here.

Suggested change

return strtolower(str_replace([' ', 'Link'], '', ClassInfo::shortName($this))) ?? '';

return strtolower(rtrim(ClassInfo::shortName($this), 'Link')) ?? '';

src/Form/Traits/AllowedLinkClassesTrait.php

maxime-rainville · 2023-12-18T04:55:23Z

src/Form/Traits/AllowedLinkClassesTrait.php

+     * Set allowed types for LinkField
+     * @param string[] $types
+     */
+    public function setAllowedTypes(array $types): static


What's our thinking when an empty array is passed to this method?

Do we want to throw an exception?

Do we want to unset any restriction and go back to allowing all Link types?

I put my comment here #137 (comment).
I think an empty array is invalid argument.

maxime-rainville · 2023-12-18T05:05:53Z

templates/SilverStripe/LinkField/Form/LinkField.ss

@@ -1,2 +1,2 @@
 <input $AttributesHTML />
-<div data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield"></div>
+<div data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProps"></div>


That doesn't need to be handle here and now, but we'll probably want to pass all our props in a single attribute here. Otherwise, every time we'll need to make our bootstrapping logic more complex to handle things like read only state.

That's more of a comment. Nothing to action at this moment.

maxime-rainville · 2023-12-18T05:08:27Z

tests/php/Services/LinkTypeServiceTest.php

+    }
+
+    /**
+     * @dataProvider allLinkTypesDataProvider


If you have only one test case, you shouldn't be using a data provider.

maxime-rainville

Happiness has been achieved.

sabina-talipova force-pushed the pulls/4/link-type-per-field-2 branch 10 times, most recently from 4dd66d5 to 7368ff8 Compare December 7, 2023 22:29

sabina-talipova marked this pull request as ready for review December 7, 2023 22:30

sabina-talipova mentioned this pull request Dec 8, 2023

Controlling link type per field #15

Closed

maxime-rainville requested changes Dec 11, 2023

View reviewed changes

sabina-talipova force-pushed the pulls/4/link-type-per-field-2 branch 4 times, most recently from 421a198 to 5a2c284 Compare December 12, 2023 01:31

maxime-rainville requested changes Dec 13, 2023

View reviewed changes

sabina-talipova force-pushed the pulls/4/link-type-per-field-2 branch 3 times, most recently from 9605dfe to 4df1552 Compare December 14, 2023 22:40

sabina-talipova force-pushed the pulls/4/link-type-per-field-2 branch 3 times, most recently from 6d0cae5 to 6981fb6 Compare December 18, 2023 01:58

maxime-rainville requested changes Dec 18, 2023

View reviewed changes

NEW Allowed link types

84824c7

sabina-talipova force-pushed the pulls/4/link-type-per-field-2 branch from 6981fb6 to 84824c7 Compare December 18, 2023 19:17

maxime-rainville approved these changes Dec 19, 2023

View reviewed changes

maxime-rainville merged commit c171ae8 into silverstripe:4 Dec 19, 2023
10 checks passed

maxime-rainville deleted the pulls/4/link-type-per-field-2 branch December 19, 2023 23:09

maxime-rainville mentioned this pull request Dec 19, 2023

NEW Set allowed link types #86

Closed

emteknetnz mentioned this pull request Dec 21, 2023

ENH Save relations on link creation #146

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NEW Allowed link types #137

NEW Allowed link types #137

sabina-talipova commented Dec 7, 2023 •

edited

Loading

maxime-rainville left a comment

maxime-rainville Dec 11, 2023

sabina-talipova Dec 11, 2023

maxime-rainville left a comment

sabina-talipova commented Dec 17, 2023 •

edited

Loading

maxime-rainville left a comment

maxime-rainville Dec 18, 2023

sabina-talipova Dec 18, 2023

maxime-rainville Dec 18, 2023

sabina-talipova Dec 18, 2023 •

edited

Loading

maxime-rainville Dec 18, 2023

maxime-rainville Dec 18, 2023

sabina-talipova Dec 18, 2023

maxime-rainville left a comment

	if (is_array($types) \|\| !empty($types)) {
	if (!empty($types)) {

	return strtolower(str_replace([' ', 'Link'], '', ClassInfo::shortName($this))) ?? '';
	return strtolower(rtrim(ClassInfo::shortName($this), 'Link')) ?? '';

NEW Allowed link types #137

NEW Allowed link types #137

Conversation

sabina-talipova commented Dec 7, 2023 • edited Loading

Description

Parent issue

maxime-rainville left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxime-rainville left a comment

Choose a reason for hiding this comment

sabina-talipova commented Dec 17, 2023 • edited Loading

maxime-rainville left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabina-talipova Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxime-rainville left a comment

Choose a reason for hiding this comment

sabina-talipova commented Dec 7, 2023 •

edited

Loading

sabina-talipova commented Dec 17, 2023 •

edited

Loading

sabina-talipova Dec 18, 2023 •

edited

Loading