Skip to content

reminder: add send to speaker option#4966

Merged
ThiefMaster merged 4 commits intoindico:masterfrom
Naveenaidu:naveen/4958-add-speakers-to-reminder
Jul 5, 2021
Merged

reminder: add send to speaker option#4966
ThiefMaster merged 4 commits intoindico:masterfrom
Naveenaidu:naveen/4958-add-speakers-to-reminder

Conversation

@Naveenaidu
Copy link
Copy Markdown
Contributor

@Naveenaidu Naveenaidu commented Jun 27, 2021

closes #4958

Screenshot of the option:
2021-06-29_22-47

TODO:

  • Apply migrations
  • Add Changelog entry
  • Fetch the speakers and add them to all_recepients
  • Format the code and apply the suggestion from pylint

@ThiefMaster
Copy link
Copy Markdown
Member

ThiefMaster commented Jun 27, 2021

Thanks! Please also add:

  • a changelog entry (for 3.0rc2 - you need to add the "Improvements" section - please position it in the same way as in other versions) referencing the issue+PR
  • the alembic revision (indico db migrate -m 'Add reminder send_to_speakers column') to create the column in an existing DB (see e.g. here for an example how to create a non-nullable column with its proper default value)

@Naveenaidu
Copy link
Copy Markdown
Contributor Author

Naveenaidu commented Jun 27, 2021

Thanks for reviewing @ThiefMaster ^^

I had a few questions regarding this. I'm still learning the codebase so please bear with me if the questions sound very naive :3

  1. Should the send_to_speaker option be visible for all the Events? If yes, who would be the speakers in the meeting event?
  2. To get the speakers for the lecture event, which part of the codebase should I look into?
  3. For the conference event. Could you point me to the documentation if available to know who is considered as the Contributors and SubContributors. The terms are a little confusing to me. Also again which part of the codebase should I look into for them? Is it the contribution.py, subcontribution.py or the person.py files? I am still trying to figure out the correlation between them

I'm kinda thinking that I would definitely have to issue a db query to get the speakers. I am just not sure from what table I need to so that ^^'

@ThiefMaster
Copy link
Copy Markdown
Member

I'd consider anyone who's a speaker anywhere in the event, including chairpersons (technically meeting/conference chairpersons are the same as lecture speakers). Probably it's a good idea to skip anything but those on the event in case the event type is lecture, just to avoid any weird surprises if someone changes event types.

For the event itself you can just iterate over the people in event.person_links.

For contributions etc. you need a query like this:

contrib_speakers = (
    ContributionPersonLink.query
    .filter(
        ContributionPersonLink.is_speaker,
        ContributionPersonLink.contribution.has(is_deleted=False, event=event)
    )
    .all()
)

Likewise for the other types. subcontributions need some more nesting, something like this (untested) should work:

subcontrib_speakers = (
    SubContributionPersonLink.query
    .filter(
        SubContributionPersonLink.is_speaker,
        SubContributionPersonLink.subcontribution.has(
            db.and_(
                ~SubContribution.is_deleted,
                SubContribution.contribution.has(is_deleted=False, event=event)
            )
        )
    )
    .all()
)

Comment thread indico/modules/events/reminders/models/reminders.py Outdated
@Naveenaidu Naveenaidu force-pushed the naveen/4958-add-speakers-to-reminder branch from 05b4427 to 110f0e7 Compare June 29, 2021 16:05
@Naveenaidu Naveenaidu marked this pull request as ready for review June 29, 2021 16:07
@Naveenaidu Naveenaidu requested a review from ThiefMaster June 29, 2021 16:07
Comment thread indico/modules/events/reminders/forms.py Outdated
Comment thread indico/modules/events/reminders/models/reminders.py Outdated
Comment thread indico/modules/events/reminders/models/reminders.py Outdated
Comment thread indico/modules/events/reminders/models/reminders.py Outdated
Comment thread CHANGES.rst Outdated
@Naveenaidu Naveenaidu force-pushed the naveen/4958-add-speakers-to-reminder branch from 110f0e7 to 4cf16f6 Compare June 29, 2021 17:20
@Naveenaidu Naveenaidu requested a review from ThiefMaster June 29, 2021 17:23
@Naveenaidu Naveenaidu force-pushed the naveen/4958-add-speakers-to-reminder branch from 4cf16f6 to 52eaeea Compare June 29, 2021 17:31
Comment thread indico/modules/events/reminders/models/reminders.py Outdated
@ThiefMaster ThiefMaster force-pushed the naveen/4958-add-speakers-to-reminder branch from 52eaeea to dd998b3 Compare July 5, 2021 11:44
@ThiefMaster ThiefMaster added this to the v3 milestone Jul 5, 2021
We care about the order...
@ThiefMaster ThiefMaster enabled auto-merge (squash) July 5, 2021 12:19
@ThiefMaster ThiefMaster merged commit fdc7e4c into indico:master Jul 5, 2021
pferreir pushed a commit to pferreir/indico that referenced this pull request Oct 12, 2021
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.

Reminders: Add "send to speakers" option

2 participants