taoceanz

Same goes for anyone critiquing me. Growth is achieved through truth.

taoceanz

Hey Dylan, for future reference, many of us feel like we need to be light in our opinions about someone else or their work as to not hurt their feelings. Just fyi, I'm not fussed what your thoughts are on my questions or code so long as you explain why. If you think my code is junk, I'm not fused so long as you explain why it's so bad and what could be done to improve it. Don't feel you need to send emoji smiley faces on my account when critiquing me. If that makes sense.

taoceanz

It's dynamically created the be set to the current locale so ends up like SiteTree_Localised_en_US or SiteTree_Localised_zh_cmn. I'd prefer not to repeat SiteTree_Localised_{$current_locale} multiple times.

  1. $current_locale = FluentState::singleton()->getLocale();
  2. $sitetree_table_localised = "SiteTree_Localised_{$current_locale}";
taoceanz

This may be sufficient perhaps

  1. $sql = implode(
  2. ' ',
  3. [
  4. 'SELECT DISTINCT SiteTree_Live.ID, SiteTree_Live.Title,',
  5. "MATCH ({$sitetreetables}) AGAINST (?) AS Relevance",
  6. 'FROM SiteTree_Live',
  7. "LEFT JOIN SiteTree_Localised AS {$sitetree_table_localised}",
  8. "ON SiteTree_Live.ID = {$sitetree_table_localised}.RecordID AND {$sitetree_table_localised}.Locale = ? AND (({$sitetree_table_localised}.ID IS NOT NULL))",
  9. "WHERE (MATCH ({$sitetreetables}) AGAINST (? IN BOOLEAN MODE) + MATCH ({$sitetreetables}) AGAINST (? IN BOOLEAN MODE) AND SiteTree_Live.ShowInSearch <> 0)",
  10. 'ORDER BY Relevance DESC',
  11. 'LIMIT 20'
  12. ]
  13. );
  14. $params = [
  15. $keywords_strrpl,
  16. $current_locale,
  17. $keywords,
  18. $htmlEntityKeywords
  19. ];
  20. $custom_results = DB::prepared_query($sql, $params);
taoceanz

That's a decent idea too. I looked at SQLQuery though wasn't sure about Distinct and Match

taoceanz

You're right Dylan, I definitely need to do some reading up on it. I'm fine to confess SQL isn't one of my strengths due to lack of time spent with it.

Very true, no updating or deletion, just getting data from it. I've never met Damian, but from the modules of his I've seen, he seems to be a human wizard. If he's saying there's a high risk of injection, I've taken that at face value and am trying to adjust my PR to ensure he's happy with it. I'll check with him about altering part of the query to be paramaterised, while table names to be variable substituted.

Not just for testing, Jack. I chose to join the query via implode rather than to using dots to concat a string. Do you think that's a bad idea?

taoceanz

This works successfully and returns search queries per locale. It's just translating some of the variables into params to pass to the prepared query that breaks it for some reason.

taoceanz

Sorry, that's not the full query. It's here for greater reference. https://github.com/taoceanz/silverstripe-fluent/blob/feature/ss4-core-search-extension/src/Extension/FluentSearchFormExtension.php#L65

Show 1 attachment(s)
GitHub  
taoceanz/silverstripe-fluent

Multi-language translate module for Silverstripe, without having to manage separate site trees. - taoceanz/silverstripe-fluent

Hide attachment content