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

Decline articles #716

Merged

Conversation

tomhatzer
Copy link
Contributor

Hey!

This fixes #580 and adds decline and undecline functionality to the articles. It adds a new nullable column to the articles table named declined_at.

Feel free to comment if you have any recommendations or if there's something missing.

Have a great day! 😃

Copy link
Contributor

@joedixon joedixon left a comment

Choose a reason for hiding this comment

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

Hi @tomhatzer, thank you very much for the contribution.

This mostly looks great, but I don't think we need the ability to undecline an article. Once it has been declined, it's typically because we don't fing the content to be appropriate.

Would it be possible to remove this functionality and I'll then review again. Thanks!

@tomhatzer
Copy link
Contributor Author

Hey @joedixon ,

I've just removed the undecline functionality.

Looking forward to your review 😃

Thank you!

app/Jobs/DeclineArticle.php Outdated Show resolved Hide resolved
app/Jobs/DeclineArticle.php Outdated Show resolved Hide resolved
tomhatzer and others added 2 commits November 3, 2021 12:08
Co-authored-by: Dries Vints <dries@vints.io>
…rning article in DeclineArticle handle method
@tomhatzer
Copy link
Contributor Author

Thanks for the review @driesvints, all changes have been implemented.

@driesvints driesvints changed the title Fix #580 - Add (un-)decline functionality Decline articles Nov 3, 2021
Copy link
Contributor

@joedixon joedixon 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 to me. I've been looking forward to this being built so thank you @tomhatzer

@driesvints driesvints merged commit caf505a into laravelio:main Nov 4, 2021
@driesvints
Copy link
Member

Thanks @tomhatzer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Decline articles
3 participants