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

Stream should be checked before tweeting #190

Open
christophrumpel opened this issue Feb 4, 2022 · 6 comments
Open

Stream should be checked before tweeting #190

christophrumpel opened this issue Feb 4, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@christophrumpel
Copy link
Owner

Today a tweet about an upcoming stream was published. The stream was already changed on Youtube, but still wrong data on Larastreamers.

Let's update a stream before we send a tweet.

@christophrumpel christophrumpel added the bug Something isn't working label Feb 4, 2022
@christophrumpel christophrumpel changed the title Tweet is not correct Stream should be checked before tweeting Feb 4, 2022
@mathiasonea
Copy link
Contributor

mathiasonea commented Feb 15, 2022

Would we do that by executing the ImportVideoAction Job with the existing stream to make sure the stream is up to date?

maybe something like this in the handle() method of the TweetStreamIsLiveJob and TweetStreamIsUpcomingJob

    $this->makeSureStreamIsUpToDate();

and its implementation down below

    protected function makeSureStreamIsUpToDate(): void
    {
        $this->stream = app(ImportVideoAction::class)->handle(
            youTubeId: $this->stream->youtube_id,
            languageCode: $this->stream->language_code,
            approved: $this->stream->isApproved(),
            submittedByEmail: $this->stream->submitted_by_email,
        );
    }

Or do you think it might make sense to introduce another Job UpdateVideoAction which receives only the youtube_id to make sure we don't have to think about the other parameters when we only want to make sure that specific Video's data is getting updated in our database?

@mathiasonea
Copy link
Contributor

mathiasonea commented Feb 15, 2022

@christophrumpel It's about the handle() methods in the TweetStreamIsLiveJob and TweetStreamIsUpcomingJob. Sorry, I had it wrong in the initial issue description.

We could re-use the ImportVideoAction because it already does what we want (update an existing stream).
But we have to pass the other parameters, (languageCode, approved, submittedByEmail) otherwise it would overwrite these.

BTW Thanks for mentioning this issue in your latest stream, it was cool seeing you trying to tackle this 💪

@christophrumpel
Copy link
Owner Author

Hey, thank YOU for tackling this in the first place :-) Yeah, let's re-use this action for now with the tweet jobs. Once it is done, we still can think of a "cleaner" solution.

Let me know if you need help with the implementation or the tests. thx Mathias

@christophrumpel
Copy link
Owner Author

Hey @mathiasonea, did you start working on this? No issue if not, please just let me know.

@mathiasonea
Copy link
Contributor

@christophrumpel no, unfortunately I didn't have as much time for this as I'd wanted to..

@christophrumpel
Copy link
Owner Author

No worries, just good to know 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants