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

Feat(hashtags): Add hashtag storage and relation syncing #495

Merged
merged 23 commits into from
Aug 25, 2024

Conversation

steven-fox
Copy link
Collaborator

@steven-fox steven-fox commented Aug 8, 2024

Our next stop in our hashtag journey is here!

This PR:

  1. Adds a new Hashtag model (with migration, factory, etc).
  2. Adds a many-to-many relationship between hashtags and questions.
  3. Syncs the parsed hashtags found in a question's content and answer upon question.created and question.updated (creating any new hashtag records when needed).
  4. Detaches hashtags upon question.deleted.

There are no UI changes with this PR (that'll come next!). This simply provides the backend processing and storage needed to produce forthcoming UI elements.

image 100% test coverage, all passing.

No phpstan errors.

I am getting some rector failures but it deals with code outside of this PR's changes...

Relates to #371

Copy link
Collaborator

@CamKem CamKem left a comment

Choose a reason for hiding this comment

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

Looks good. We probably didn't need a new model / table, parsing would have been enough, however storing the hashtags will mean we can introduce analytics & similar down the track.

Can we add a way to filter the feed by hashtag? Such as:

// Route:
Route::get('/tag/{hashtag}', HashtagController::class)->name('home.hashtag');

// Controller:
class HashtagController
{

    public function __invoke(string $hashtag)
    {
        return view('hashtag.show', [
            'hashtag' => $hashtag,
        ]);
    }

}

// View:
<x-app-layout>
    <x-slot name="title">Posts with #{{ $hashtag }}</x-slot>

    <div class="flex flex-col items-center justify-center">
        <div class="w-full max-w-md overflow-hidden rounded-lg px-2 shadow-md sm:px-0">
            <x-home-menu></x-home-menu>

            @auth
                <livewire:questions.create :toId="auth()->id()" />
            @endauth

            <livewire:home.feed :hashtag="$hashtag" />
        </div>
    </div>
</x-app-layout>

Then we just need to add a property to the Feed component.

 public ?string $hashtag = null;
 
 // Use the property in to filter the feed.
->when($this->hashtag, fn ($query) => $query
    ->whereHas('hashtags', fn ($query) => $query
        ->where('name', $this->hashtag)
    )
)

@steven-fox
Copy link
Collaborator Author

Thanks for the review Cam!

I do think a model + relations is the way to go here, for a few reasons:

  1. Just searching the database for ->where(‘answer’, ‘like’, ‘%$hashtag%’) isn’t sufficient, I don’t think, as that would return questions that include the # symbol in places we don’t care about (like urls). So that’d leave regex searching, and it’ll quickly become very slow (no use of indexes are possible - even a full text search index).
  2. I wanted to be able to provide the autocompletion of hashtags with the count of questions related to each hashtag name so that a user can decide on a best choice from similar options.
  3. Then, as you mentioned, additional analytics/querying becomes easier/possible.

Last but not least, thanks for the blueprint of UI/feed related stuff! As I mentioned up top, the purpose of this PR is solely backend operations to keep it easier to review and break up potentially bugs into separate reverts haha. UI stuff comes next 😁

app/Services/QuestionHashtagSyncer.php Outdated Show resolved Hide resolved
app/Services/QuestionHashtagSyncer.php Outdated Show resolved Hide resolved
app/Services/QuestionHashtagSyncer.php Outdated Show resolved Hide resolved
@steven-fox
Copy link
Collaborator Author

@MrPunyapal I updated this PR based on our discussion:

  • The _ is no longer permitted in hashtags.
  • Hashtags are limited to 50 characters in length. Can be adjusted in the future but this seems like a sane starting point.
  • I added a collate nocase index for hashtags.name. That's all that's needed for me to handle the UI features we discussed in a db-efficient manner.

I believe this is good to go but please give it another look and let me know if you spot any loose ends.

I can follow up soon after with the UI PR.

@nunomaduro nunomaduro merged commit d23a3c2 into main Aug 25, 2024
@nunomaduro nunomaduro deleted the feat/hashtag-storage branch August 25, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants