feat: add base and learner portal bff handler

[brobro10000]

Description:
Adds a BaseHandler which utilizes the HandlerContext responsible for the underlying structure for users of the BaseHandler.

Adds the BaseLearnerPortalHandler defines respective usable clients and loads data within the HandlerContext
It is the orchaestration layer for the MFE responsible for making the API calls and performing transformations. Currently the functions in this class are stubbed out to be completed in future work.

Adds the DashboardHandler which retrieves the data from the context to be consumed by the to be added ResponseBuilder

Adds MVP tests. The functions and tests will be iterated on specifically on within their separate pieces of work.

Jira:
ENT-XXXX

Merge checklist:

• ./manage.py makemigrations has been run
  • Note: This must be run if you modified any models.
    • It may or may not make a migration depending on exactly what you modified, but it should still be run.

Post merge:

• Ensure that your changes went out to the stage instance
• Deploy to prod instance

[adamstankiewicz]

Looking great, left a couple suggestions to consider and a few nits.

[adamstankiewicz]

I believe this line still isn't required, as the BaseHandler already sets self.context in the previous super().__init__(context) line.

[brobro10000]

Removing it presents the following linting error

[adamstankiewicz]

Which line did you remove? It looks like you removed super().__init__(context), not self.context = context?

[adamstankiewicz]

[suggestion] I'm not quite sure if we truly need to conditionally call self.load_subscription_licenses here vs. assuming we've already loaded subscription licenses metadata prior to calling get_subscription_licenses. Given get_subscription_licenses and get_subscription_licenses_by_status are only executed via self.process_subscription_licenses, perhaps an improvement would be to instead have some validation within self.process_subscription_licenses to raise an exception and/or log if self.context.data['subscriptions'] is not set (i.e., fail early before attempting to process licenses, whereby these methods wouldn't be called).

The potential concern is that it's setting up a pattern where we need to have a side effect of load_subscription_licenses anytime we need to retrieve data about subscriptions (e.g., having to do this in 2+ places, not super DRY) vs. enforcing these methods don't get called and/or raising an exception within self.process_subscription_licenses when subscriptions doesn't yet exist in the context data.

[adamstankiewicz]

As is, in the case where self.load_subscription_licenses is called, I don't believe this line would properly have the updated context data. For example, if we wanted to have the side effect of load_subscription_licenses, would it need to be something like the following:

if not (subscriptions := self.context.data['subscriptions']):
    self.load_subscription_licenses()

# Retrieve subscriptions data from the context again, now that it's been updated via `load_subscription_licenses`.
subscriptions = self.context.data['subscriptions']
return subscriptions.get('subscription_licenses', [])

[adamstankiewicz]

Similar feedback as get_subscription_licenses.

[adamstankiewicz]

nit:

Suggested change

	# Check if there user should be auto-applied a license
	# Check if the user should be auto-applied a license

[adamstankiewicz]

nit: perhaps rename slightly to suggest testing parsing params.

Suggested change

	def test_handler_context_enterprise_customer_uuid(self, query_params, data):
	def test_handler_context_enterprise_customer_uuid_param(self, query_params, data):

[adamstankiewicz]

[nit/curious] Would it make sense to rely on self.context here as it's already instantiated in the setUp?

self.assertEqual(self.context.request, self.request)
# ...

[adamstankiewicz]

nit: I'm not sure if it makes sense to initialize the mock request with both query_params AND data simultaneously, as typically it'd be either one or the other.

[brobro10000]

You are correct but there is a test that checks for whether the query params or data is included. Selectively removing them from the request from setUp makes for a cleaner test versus either not defining them at all and having to explicitly define it per test, or leaving one of them, and parsing which one exist and which one does not.

  @ddt.data(
        {
            'query_params': True,
            'data': True,
        },
        {
            'query_params': False,
            'data': True,
        },
        {
            'query_params': True,
            'data': False,
        },
        {
            'query_params': False,
            'data': False,
        }
    )
    @ddt.unpack
    def test_handler_context_enterprise_customer_uuid(self, query_params, data):
        if not query_params:
            del self.request.query_params['enterprise_customer_uuid']
        if not data:
            del self.request.data['enterprise_customer_uuid']
        request = self.request
        if not (query_params or data):
            with self.assertRaises(ValueError):
                HandlerContext(request)
        else:
            context = HandlerContext(request)
            self.assertEqual(context.enterprise_customer_uuid, self.mock_enterprise_customer_uuid)
            self.assertEqual(context.lms_user_id, self.mock_user.lms_user_id)

[adamstankiewicz]

I think these extending a TestBaseHandlerMixin came out nicely :)