check ranking command and jobs #82
Conversation
src/Models/Keyword.php
Outdated
| @@ -23,6 +24,11 @@ protected static function boot() | |||
| static::saving(function ($keyword) { | |||
| $keyword->phrase = strtolower($keyword->phrase); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
Are we sure we want to add this as a global scope? It seems like this should just be a customer query builder or local scope since the user is going to want to see their inactive keywords somewhere
There was a problem hiding this comment.
I was trying to do it like the locations package. But I think you are allowed to do something like static:: withoutGlobalScope('active')->get() right?
- add active scope to keyword model - add check keyword rankings commands - add shorcuts to actions inside of keyword and ranking models - update CheckRankingCommand.php to use action - add an exception to GetOrganicResults.php so we know when it doesn't return any data - add setActive method to Keyword, which updates appropriate date fields
|
I have removed the unique constraint from the results table, it doesn't work for sitelinks which would all be the same as technically (e.g. attached image - rows 150-153 are all parent and sitelinks to wikipedia results). Also, I was able to test the local and organic results individually but the chain isn't working so the local results dont get executed after the organic. Maybe it's too late and I need to look at it again in the morning 😅 |
| @@ -14,6 +14,7 @@ public function up() | |||
| $table->id(); | |||
| $table->string('phrase')->index()->unique(); | |||
| $table->string('type')->index(); // Example: 'Branded', 'Generic', 'Local' | |||
| $table->boolean('is_active')->index(); | |||
There was a problem hiding this comment.
I would prefer to not have this in the database. Can we just create a scope for the active keywords based on the datetime fields for tracking_requested_at and tracking_stopped_at?
There was a problem hiding this comment.
Yes I will put that back.
There was a problem hiding this comment.
Thanks!
Let's discuss with the team and we may want to add this back if there are performance gains.
There was a problem hiding this comment.
Sorry @drewroberts I had asked Arwa to add this since we are going to be using that active scope for everything in regards to keywords. Yes I was thinking it would be better for performance but that just depends on how big the keyword database will get. If it's not going to get that big then it probably wouldn't make much of a difference. I am curious to know the others' opinions on it though.
| @@ -22,7 +22,7 @@ public function up() | |||
|
|
|||
| // Don't need timestamps since can use created_at timestamp of the ranking class | |||
|
|
|||
| $table->unique(['ranking_id', 'type', 'position'], 'result_unique'); | |||
| //$table->unique(['ranking_id', 'type', 'position'], 'result_unique'); | |||
There was a problem hiding this comment.
I don't think sitelinks should prevent the use of unique results. They should have a different type value.
There was a problem hiding this comment.
I am not sure I understand. All the sitelinks of a website would have the same ranking id and position which they would get from the parent webpage. Should we have a "Sitelink" type enum for the "type" column? But even so, all sitelinks would have that same type. So the combination of the 3 would still be duplicated for the sitelinks.
There was a problem hiding this comment.
OK, I'm going to push this through and then let's take a look at how it runs in a Zoom meeting and I'll know more then what we should do. My thought is that I want to know what order the sitelinks are in. They should have a parent_id of the main result and be a different type with their own ranking.
There was a problem hiding this comment.
Oh! That makes sense! I think I get it now. However, sitelinks results don't have a "position" unlike local, organic, etc. We could use their position in the result set itself, or the "key" of the array element.
There was a problem hiding this comment.
Let's run some examples on their website and see if the key matches the order in which they are shown in the search results.
| $table->id(); | ||
| $table->foreignIdFor(app('keyword'))->index(); | ||
| $table->foreignIdFor(app('search_locale'))->index(); | ||
| $table->timestamps(); |
There was a problem hiding this comment.
Let's add creator & updater to this so we know who added them.
Would appreciate some feedback to see what needs to be changed. Thanks!