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

Exclude redundant notifications when inforg edits a search topic on the forum #13

Open
AKagitin opened this issue Jul 9, 2023 · 6 comments

Comments

@AKagitin
Copy link
Contributor

AKagitin commented Jul 9, 2023

We can reduce number of notification messages in this case and send 1 instead of 2.
image

If we register notification of "обновление заголовка поиска по" type then we could possibly discover that this change is equal to change of status of this search and so we could add "... - изменение статуса по" to this notification and exclude additional notification "изменение статуса по" relating to this change.

@AKagitin
Copy link
Contributor Author

@Mikkdrasil Please advise how can I identify which line with change_type==1 ('status_change') is corresponding to a line with change_type==2 ('title_change') currently been examined.

@Mikkdrasil
Copy link
Collaborator

@AKagitin i guess "which line" is a too broad meaning. Depends in which script / which PSQL table we're checking it.
If you'd like to exclude "doubling" of the same "news" (like НЖ) in different message types – you'd need to create a comprehensive logic to support it. I guess script compose_notifications should parse texts of the key message types (like title change, first post change, comment change) – and write parsed "status" to the new filed in notif_by_user table. Then during it's work compose_notifications should check this column for other records – if same search_id, user_id and this parsed_status are already exist (assuming in the nearest time-radius only) – not to compose a new message. Here we also should have hierarchy: which update type should be composed first.

..so all-in-all looks like a big change.

@AKagitin
Copy link
Contributor Author

I guess script compose_notifications should parse
Hello, @Mikkdrasil. Sorry, I have know time to dig in code. As I remember compose_notifications process records in DB which produced by another script which actually parses the forum messages. May be better solution would be to modify the parsing script so that if it parses the search status change (aka 'НЖ') then it skips parsing of some other type of changes in this search?

@AKagitin
Copy link
Contributor Author

remarks for myself:

  1. compose_new_records_from_change_log берет 1 строку из change_log. Интересные поля: status, change_type == 1 (status change)
  2. compose_com_msg_on_status_change заполняет текст f'{status_info} – изменение статуса по {clickable_name}' в line.message

@Mikkdrasil
Copy link
Collaborator

Mikkdrasil commented Jan 29, 2024

I guess script compose_notifications should parse
Hello, @Mikkdrasil. Sorry, I have know time to dig in code. As I remember compose_notifications process records in DB which produced by another script which actually parses the forum messages. May be better solution would be to modify the parsing script so that if it parses the search status change (aka 'НЖ') then it skips parsing of some other type of changes in this search?

sorry, my friend, seems i'm keeping missing the updates here (would need to think how to avoid it in the future..)
that is correct that [compose_notifications] is indeed invoked by either [identify_updates_of_first_posts] or [identify_updates_of_topics].

not sure i understand what does "stop parsing" means.
you first download the whole page, then you parse it, then you work with the content you parsed as you prefer.

if i hear you correctly, you'd like not to skip parsing, but to skip recording some new changes to change_log table in DB?
if so, my vision was that it's already somehow implemented. or pls suggest the more focused question to understand your idea in better details. thanks
@AKagitin

@AKagitin
Copy link
Contributor Author

AKagitin commented Apr 9, 2024

In SQL statement in compose_users_list_from_users we can detect a situation when new_record is redundant for a user because user is subscribed to inforg's comments and new_record.change_type=2(topic_title_change) because this change is always made only by inforg and causes also inforg's comments. Such users should be skipped for such new_record.

So here is the modification:

WHERE 30 = ANY(agg) OR :a = ANY(agg)),

Pull request: #422

AKagitin added a commit to AKagitin/la_searcher_bot_ak that referenced this issue Apr 9, 2024
AKagitin added a commit to AKagitin/la_searcher_bot_ak that referenced this issue Apr 9, 2024
Mikkdrasil added a commit that referenced this issue May 1, 2024
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

No branches or pull requests

2 participants