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: Credit Username on Approval #162

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

LightDestory
Copy link
Member

@LightDestory LightDestory commented Feb 25, 2024

Prerequisites

  • I have read and understood the contributing guide.
  • The commit message follows the conventional commits guidelines.
  • Tested the changes locally with a real Telegram bot.
  • Introduced tests for the changes have been added (for bug fixes / features).
  • Docs have been added/updated (for bug fixes / features).
  • I have updated the CHANGELOG.rst file with an overview of the changes made.

Description

This PR is a re-implementation of the now abandoned #114.

It follows a new approach: save the credited username when creating the PendingPost object.

image

(If applicable) Issue closed by this PR

Does this PR introduce a breaking change?

  • Yes
  • No

(If yes) What are the changes that might break existing applications?

The Pending Post database entity has been updated. Migrations script should be performed.

Python version you are using

Python 3.12.0

Copy link
Member

@TendTo TendTo left a comment

Choose a reason for hiding this comment

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

For the migration, we can use the following script

--- migration.sql
-- Add column credit_username to table pending_posts
ALTER TABLE pending_post ADD COLUMN credit_username VARCHAR(255) DEFAULT NULL;

to run with run_sql migration.sql spotted.sqlite3


While the approach is correct, it introduces some inconsistencies with the current method. Right now the credited username is picked when the spot is sent to the channel.

sign = await User(user_id).get_user_sign(bot=self.__bot)

meaning that if the user was not credited before spotting but becomes credited before the admins approve the spot, their username will appear without the admin's knowledge.
The same reasoning applies to the username: it could be changed during the approval time.

src/spotted/data/pending_post.py Outdated Show resolved Hide resolved
@LightDestory LightDestory requested a review from TendTo February 25, 2024 19:30
@Helias
Copy link
Member

Helias commented Feb 26, 2024

merge conflict

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.

Show username during approval when message is with credits
3 participants