[PY] chore: refactor _on_turn

[BMS-geodev]

Linked issues

closes: #1559

Details

Broke up the _on_turn() method for clarity. The logical parts of the large functionality were broken into separate neighboring private methods each of which having more atomic responsibilities.

The behavior of _on_turn() should remain consistent, but in general be more readable.

Change details

N/A

code snippets: Here are some sample adjustments.

ex. remove mentions:

async def _on_turn(self, context: TurnContext):
...
    # remove @mentions
    if self.options.remove_recipient_mention:
        if context.activity.type == ActivityTypes.message:
            context.activity.text = context.remove_recipient_mention(context.activity)

is now:

async def _on_turn(self, context: TurnContext):
...
    await self._remove_mentions(context)

async def _remove_mentions(self, context: TurnContext):
    if self.options.remove_recipient_mention and context.activity.type == ActivityTypes.message:
        context.activity.text = context.remove_recipient_mention(context.activity.text)

ex. running the AI chain when matches are not found:

async def _on_turn(self, context: TurnContext):
...
    # only run chain when no activity handlers matched
    if (
        matches == 0
        and self._ai
        and self._options.ai
        and context.activity.type == ActivityTypes.message
        and context.activity.text
    ):
        is_ok = await self._ai.run(context, state)

        if not is_ok:
            await state.save(context, self._options.storage)
            return

is now:

async def _on_turn(self, context: TurnContext):
    if matches == 0:
      if not await self._run_ai_chain(context, state):
          return

async def _run_ai_chain(self, context: TurnContext, state):
    if (
        self._ai
        and self._options.ai
        and context.activity.type == ActivityTypes.message
        and context.activity.text
    ):
        is_ok = await self._ai.run(context, state)
        if not is_ok:
            await self._save_state(context, state)
            return False
    return True

screenshots:

Attestation Checklist

• [X] My code follows the style guidelines of this project

• I have checked for/fixed spelling, linting, and other errors

• I have commented my code for clarity

• I have made corresponding changes to the documentation (updating the doc strings in the code is sufficient)

• My changes generate no new warnings

• I have added tests that validates my changes, and provides sufficient test coverage. I have tested with:

  • Local testing
  • E2E testing in Teams

• New and existing unit tests pass locally with my changes

Additional information

Feel free to add other relevant information below

[BMS-geodev]

Naming things is tough, especially when new to a codebase. Please let me know if any naming choices clash with any style guidelines or intended vocabulary.

[BMS-geodev]

@microsoft-github-policy-service agree company="Awarity Inc."

[BMS-geodev]

Local testing, as well as poetry run test both positive.

[BMS-geodev]

@corinagum , I pulled the most recent changes from main and locally ran poetry run test and got all successful. I believe the PR is in a good state, so much as I can see.

[BMS-geodev]

still chasing updates on other branches, ensuring code here is synced

[BMS-geodev]

After merging in the state, and auth changes from other PR's, and making the needed adjustments, I now have working poetry tests, as well as samples behaving as expected.

I believe this PR is ready for review again.