isobar-nz / silverstripe-seo-editor Goto Github PK

The handleAction function does a direct UPDATE on the database to gain speed:

DB::query("UPDATE SiteTree SET {$fieldName} = '{$sqlValue}' WHERE ID = {$page->ID}");
if ($page->isPublished()) {
     DB::query("UPDATE SiteTree_Live SET {$fieldName} = '{$sqlValue}' WHERE ID = {$page->ID}");
}

This causes other modules like Fluent to work incorrectly.
If you change this to the ORM method:

$page->$fieldName = $sqlValue;
$page->write();
if ($page->isPublished()) {
      $page->doPublish();
}

I made a fork with a new branch called enable-augmentation, with this implemented:
https://github.com/RVXD/silverstripe-seo-editor/tree/enable-augmentation

Perhaps if you would want the increased speed for sites without Fluent, this could be made optional.

Hi,
I saw there is a separate repository for working with Fluent, but this is for SilverStripe 3 only. Is there any solution for SilverStripe 4?

Ran into a small issue today on a site using Translatable.

Translatable adds a query string which is visible in the url attribute of a gridfield fieldset. This proves problematic as this module appends to the url attribute value and we end up with an ajax post request like
"admin/seo-editor/SiteTree/EditForm/field/SiteTree?Locale=en_NZ/update/1" which results in a bad request.

To get around it quickly i've just made an extension and included a tweaked javascript file where it deals with the creating the request url.

var url = $this.closest('.ss-gridfield').attr('data-url');
if(url.indexOf('?Locale=') != -1) {
    url = url.replace('?Locale=', '/update/' + id + '?Locale=');
} else {
    url = url + "/update/" + id;
}

Was done in a bit of rush so i'm sure there's a nicer way of dealing with this. Feel free to close if it's considered something not worth worrying about.

this would be awesome to integrate with tractorcow/silverstripe-opengraph and be able to override opengraph on all pages

e.g. News & Blog

Great module. I am experiencing an issue importing a CSV file - get "Nothing to import" message after importing.

Hey @stevie-mayhew, what's up 😀

Just trying this module, it looks handy. I was thinking though, that in real world use (for me at least) all my pages have a meta title whether I provide a custom one or not, because I'll generate one if not provided. I'll sometimes do the same for Meta Descriptions too. So I would like error messages to reflect the actual tags that are output, and I would like to see the default content when a field is blank, to know whether it's worth overriding it with a custom value.

What do you think about providing a method like this to determine a default value:

// Provide a default meta title if no custom one is set
public function defaultMetaTitle() {
    return $this->Title . ' → ' . SiteConfig::current_site_config()->Title;
}

This could be overridden in the Page class to let developers define their own default. Then you could set a placeholder on form fields so that any blank fields display their default values, and show error messages based on the final resolved value rather than the custom value only.

To explain my approach further - to bring this all together I would then put something like this in my Page class:

// Resolve the actual meta title content that will be output
public function resolvedMetaTitle() {
    return $this->MetaTitle ?: $this->defaultMetaTitle();
}

And I'd put this in my PageController. (Putting it on the model would cause the resolved value to be populated on MetaTitle form fields)

// Resolve a meta title for this page
public function getMetaTitle() {
    return $this->resolvedMetaTitle();
}

When trying to access the SEO editor for a site with only one page, SeoEditorAdmin#markDuplicates generates a filter that leads to an invalid SQL query:

SELECT DISTINCT "SiteTree_Live"."ClassName", "SiteTree_Live"."Created",
"SiteTree_Live"."LastEdited", "SiteTree_Live"."URLSegment", "SiteTree_Live"."Title", 
"SiteTree_Live"."MenuTitle", "SiteTree_Live"."Content", "SiteTree_Live"."MetaDescription", 
"SiteTree_Live"."ExtraMeta", "SiteTree_Live"."ShowInMenus", "SiteTree_Live"."ShowInSearch", 
"SiteTree_Live"."Sort", "SiteTree_Live"."HasBrokenFile", "SiteTree_Live"."HasBrokenLink", 
"SiteTree_Live"."ReportClass", "SiteTree_Live"."CanViewType", "SiteTree_Live"."CanEditType", 
"SiteTree_Live"."Priority", "SiteTree_Live"."MetaTitle", "SiteTree_Live"."Version", 
"SiteTree_Live"."ParentID", "SiteTree_Live"."ID", CASE WHEN "SiteTree_Live"."ClassName" 
IS NOT NULL
THEN "SiteTree_Live"."ClassName" ELSE 'SiteTree' END AS "RecordClassName"
FROM "SiteTree_Live" WHERE ("SiteTree_Live"."ID" IN ('')) ORDER BY "SiteTree_Live"."Sort" ASC

Specifically WHERE ("SiteTree_Live"."ID" IN ('')) uses an empty string when it should be a list of integers.