From 7e2ec0b5dec5d689a35c28c28c0bb42e11f4b939 Mon Sep 17 00:00:00 2001 From: James Kiger <68701146+jamesrkiger@users.noreply.github.com> Date: Wed, 18 Dec 2024 09:31:57 -0500 Subject: [PATCH 1/6] fix(billing): check default plan limits on backend TASK-1364 (#5367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Updates backend code to check Stripe products for default "community plan" limits when determining organization usage. ### 📖 Description Currently, the frontend gets billing usage limits for the free Community Plan from the corresponding Stripe product (returned by the products endpoint). The backend, however, does not reference these limits at all. This has not been a problem previously, because the backend currently does not enforce usage limits (only NLP limits are enforced, and this is handled by the frontend at present) and had no other need for these limits. Now that the backend is tracking addon usage, however, it does need to be aware of community plan usage limits. This PR updates the get_organization_plan_limit util function to use the limits from the metadata of the default community plan when an organization does not have a subscription. ### 👀 Preview steps Change covered by unit test --- .../stripe/tests/test_organization_usage.py | 7 +- kobo/apps/stripe/tests/utils.py | 20 ++++++ kobo/apps/stripe/utils.py | 71 +++++++++---------- 3 files changed, 56 insertions(+), 42 deletions(-) diff --git a/kobo/apps/stripe/tests/test_organization_usage.py b/kobo/apps/stripe/tests/test_organization_usage.py index bd104d1b62..55e9f29a6b 100644 --- a/kobo/apps/stripe/tests/test_organization_usage.py +++ b/kobo/apps/stripe/tests/test_organization_usage.py @@ -18,6 +18,7 @@ from kobo.apps.organizations.models import Organization, OrganizationUser from kobo.apps.stripe.constants import USAGE_LIMIT_MAP from kobo.apps.stripe.tests.utils import ( + generate_free_plan, generate_mmo_subscription, generate_plan_subscription, ) @@ -461,11 +462,11 @@ def setUp(self): self.organization.add_user(self.anotheruser, is_admin=True) def test_get_plan_community_limit(self): - generate_mmo_subscription(self.organization) + generate_free_plan() limit = get_organization_plan_limit(self.organization, 'seconds') - assert limit == 2000 # TODO get the limits from the community plan, overrides + assert limit == 600 limit = get_organization_plan_limit(self.organization, 'characters') - assert limit == 2000 # TODO get the limits from the community plan, overrides + assert limit == 6000 @data('characters', 'seconds') def test_get_suscription_limit(self, usage_type): diff --git a/kobo/apps/stripe/tests/utils.py b/kobo/apps/stripe/tests/utils.py index e7dd249d86..e7dc6b61b6 100644 --- a/kobo/apps/stripe/tests/utils.py +++ b/kobo/apps/stripe/tests/utils.py @@ -8,6 +8,26 @@ from kobo.apps.organizations.models import Organization +def generate_free_plan(): + product_metadata = { + 'product_type': 'plan', + 'submission_limit': '5000', + 'asr_seconds_limit': '600', + 'mt_characters_limit': '6000', + 'storage_bytes_limit': '1000000000', + } + + product = baker.make(Product, active=True, metadata=product_metadata) + + baker.make( + Price, + active=True, + recurring={'interval': 'month'}, + unit_amount=0, + product=product, + ) + + def generate_plan_subscription( organization: Organization, metadata: dict = None, diff --git a/kobo/apps/stripe/utils.py b/kobo/apps/stripe/utils.py index c6dbb45381..7cc4e6cc33 100644 --- a/kobo/apps/stripe/utils.py +++ b/kobo/apps/stripe/utils.py @@ -5,7 +5,7 @@ from kobo.apps.organizations.models import Organization from kobo.apps.organizations.types import UsageType -from kobo.apps.stripe.constants import ACTIVE_STRIPE_STATUSES, USAGE_LIMIT_MAP +from kobo.apps.stripe.constants import USAGE_LIMIT_MAP def generate_return_url(product_metadata): @@ -30,51 +30,44 @@ def get_organization_plan_limit( organization: Organization, usage_type: UsageType ) -> int | float: """ - Get organization plan limit for a given usage type + Get organization plan limit for a given usage type, + will fall back to infinite value if no subscription or + default free tier plan found. """ if not settings.STRIPE_ENABLED: - return None - stripe_key = f'{USAGE_LIMIT_MAP[usage_type]}_limit' - query_product_type = ( - 'djstripe_customers__subscriptions__items__price__' - 'product__metadata__product_type' - ) - query_status__in = 'djstripe_customers__subscriptions__status__in' - organization_filter = Organization.objects.filter( - id=organization.id, - **{ - query_status__in: ACTIVE_STRIPE_STATUSES, - query_product_type: 'plan', - }, - ) - - field_price_limit = ( - 'djstripe_customers__subscriptions__items__' f'price__metadata__{stripe_key}' - ) - field_product_limit = ( - 'djstripe_customers__subscriptions__items__' - f'price__product__metadata__{stripe_key}' - ) - current_limit = organization_filter.values( - price_limit=F(field_price_limit), - product_limit=F(field_product_limit), - prod_metadata=F( - 'djstripe_customers__subscriptions__items__price__product__metadata' - ), - ).first() + return inf + + limit_key = f'{USAGE_LIMIT_MAP[usage_type]}_limit' + relevant_limit = None - if current_limit is not None: - relevant_limit = current_limit.get('price_limit') or current_limit.get( - 'product_limit' + if subscription := organization.active_subscription_billing_details(): + price_metadata = subscription['price_metadata'] + product_metadata = subscription['product_metadata'] + price_limit = price_metadata[limit_key] if price_metadata else None + product_limit = product_metadata[limit_key] if product_metadata else None + relevant_limit = price_limit or product_limit + else: + from djstripe.models.core import Product + + # Anyone who does not have a subscription is on the free tier plan by default + default_plan = ( + Product.objects.filter( + prices__unit_amount=0, prices__recurring__interval='month' + ) + .values(limit=F(f'metadata__{limit_key}')) + .first() ) - if relevant_limit is None: - # TODO: get the limits from the community plan, overrides - relevant_limit = 2000 - # Limits in Stripe metadata are strings. They may be numbers or 'unlimited' + + if default_plan: + relevant_limit = default_plan['limit'] + if relevant_limit == 'unlimited': return inf - return int(relevant_limit) + if relevant_limit: + return int(relevant_limit) + + return inf def get_total_price_for_quantity(price: 'djstripe.models.Price', quantity: int): From 29fabbdc6a993259d73647959dcb6b68f1b8cad1 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 18 Dec 2024 09:51:08 -0500 Subject: [PATCH 2/6] refactor(projectHistoryLogs): use audit log base class for DataViewSet TASK-1340 (#5368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 💭 Notes Internal changes only ### Preview steps 1. ℹī¸ have account and a project 2. add submissions 4. đŸŸĸ Make sure you can still see submissions in Project > Data --- kpi/views/v2/data.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kpi/views/v2/data.py b/kpi/views/v2/data.py index 19d60b35b2..7f97548bc4 100644 --- a/kpi/views/v2/data.py +++ b/kpi/views/v2/data.py @@ -8,7 +8,7 @@ from django.http import Http404, HttpResponseRedirect from django.utils.translation import gettext_lazy as t from pymongo.errors import OperationFailure -from rest_framework import renderers, serializers, status, viewsets +from rest_framework import renderers, serializers, status from rest_framework.decorators import action from rest_framework.pagination import _positive_int as positive_int from rest_framework.request import Request @@ -17,6 +17,7 @@ from rest_framework_extensions.mixins import NestedViewSetMixin from kobo.apps.audit_log.audit_actions import AuditAction +from kobo.apps.audit_log.base_views import AuditLoggedViewSet from kobo.apps.audit_log.models import AuditLog, AuditType from kobo.apps.openrosa.libs.utils.logger_tools import http_open_rosa_error_handler from kpi.authentication import EnketoSessionAuthentication @@ -54,7 +55,7 @@ class DataViewSet( - AssetNestedObjectViewsetMixin, NestedViewSetMixin, viewsets.GenericViewSet + AssetNestedObjectViewsetMixin, NestedViewSetMixin, AuditLoggedViewSet ): """ ## List of submissions for a specific asset @@ -330,6 +331,8 @@ class DataViewSet( ) permission_classes = (SubmissionPermission,) pagination_class = DataPagination + log_type = AuditType.PROJECT_HISTORY + logged_fields = [] @action(detail=False, methods=['PATCH', 'DELETE'], renderer_classes=[renderers.JSONRenderer]) From 424311f5ec512097bc4cddadc832acc50bb7b54f Mon Sep 17 00:00:00 2001 From: James Kiger <68701146+jamesrkiger@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:04:15 -0500 Subject: [PATCH 3/6] fix(organizations): limit my org projects query to surveys TASK-1389 (#5378) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Adds query param to asset API request on `myOrgProjectsRoute` to restrict query to only surveys. ### 👀 Preview steps 1. Create an MMO and a collection 2. View MMO org project list 3. On main, the collection will appear on this list 4. On this branch, the collection will not appear (because a query param is added to the API request) --- jsapp/js/projects/myOrgProjectsRoute.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jsapp/js/projects/myOrgProjectsRoute.tsx b/jsapp/js/projects/myOrgProjectsRoute.tsx index 6ae77d46a2..e0bb3ea740 100644 --- a/jsapp/js/projects/myOrgProjectsRoute.tsx +++ b/jsapp/js/projects/myOrgProjectsRoute.tsx @@ -43,7 +43,7 @@ export default function MyOrgProjectsRoute() { viewUid={ORG_VIEW.uid} baseUrl={`${ROOT_URL}${apiUrl}`} defaultVisibleFields={HOME_DEFAULT_VISIBLE_FIELDS} - includeTypeFilter={false} + includeTypeFilter defaultOrderableFields={HOME_ORDERABLE_FIELDS} defaultExcludedFields={HOME_EXCLUDED_FIELDS} isExportButtonVisible={false} From b6a2829cd8d772497f16ddd96675832333c9ecad Mon Sep 17 00:00:00 2001 From: Philip Edwards Date: Wed, 18 Dec 2024 13:11:20 -0500 Subject: [PATCH 4/6] ci(darker): improve comparison refs for focused errors (#5311) Change which commits we compare in darker.yml, not to include changes introduced by other branches More details: - https://github.com/kobotoolbox/kpi/pull/5311 - https://github.com/kobotoolbox/kpi/pull/5282#issuecomment-2489489645 --- .github/workflows/darker.yml | 37 ++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/.github/workflows/darker.yml b/.github/workflows/darker.yml index d5b221d63c..1b3a194572 100644 --- a/.github/workflows/darker.yml +++ b/.github/workflows/darker.yml @@ -6,9 +6,12 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: - fetch-depth: 0 + # In addition to checking out the 'merge commit', we also want to + # fetch enough commits to find the most recent commit in the base + # branch (typically 'main') + fetch-depth: 100 - name: Set up Python uses: actions/setup-python@v5 @@ -18,12 +21,30 @@ jobs: - name: Install pip dependencies run: python -m pip install darker[isort] flake8 flake8-quotes isort --quiet - # use `--ignore=F821` to avoid raising false positive error in typing - # annotations with string, e.g. def my_method(my_model: 'ModelName') - - # darker still exit with code 1 even with no errors on changes - - name: Run Darker with base commit + - name: Run Darker, comparing '${{github.ref_name}}' with the latest in '${{github.event.pull_request.base.ref}}' run: | - output=$(darker --check --isort -L "flake8 --max-line-length=88 --extend-ignore=F821" kpi kobo hub -r ${{ github.event.pull_request.base.sha }}) + # Run darker only on file changes introduced in this PR. + # Allows incremental adoption of linter rules on a per-file basis. + + # Prevents this scenario: + # - PR A is opened, modifying file A; CI passes. + # - PR B is opened & merged, with linter errors in file B. + # - PR A is updated again, modifying file A. CI fails from file B. + # Get the latest commit in the base branch (usually 'main') at time of + # CI run, to compare with this PR's merge branch. + # GitHub doesn't provide a nice name for this SHA, but we can find it: + # https://www.kenmuse.com/blog/the-many-shas-of-a-github-pull-request/#extracting-the-base-sha + MERGE_PARENTS=($(git rev-list --parents -1 ${{ github.sha }})) + LATEST_IN_BASE_BRANCH=${MERGE_PARENTS[1]} + + # Run darker. (https://github.com/akaihola/darker) + # -L runs the linter + # `--ignore=F821` avoids raising false positive error in typing + # annotations with string, e.g. def my_method(my_model: 'ModelName') + # -r REV specifies a commit to compare with the worktree + output=$(darker --check --isort -L "flake8 --max-line-length=88 --extend-ignore=F821" kpi kobo hub -r $LATEST_IN_BASE_BRANCH) + + # darker still exits with code 1 even with no errors on changes + # So, make this fail CI only if there is output from darker. [[ -n "$output" ]] && echo "$output" && exit 1 || exit 0 shell: /usr/bin/bash {0} From 33066238da5fb8c113722596804656b0f2a1402d Mon Sep 17 00:00:00 2001 From: Paulo Amorim Date: Thu, 19 Dec 2024 09:09:36 -0300 Subject: [PATCH 5/6] feat(projectHistoryLogs): remove activity logs feature flag TASK-1385 (#5364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 🗒ī¸ Checklist 1. [x] run linter locally 2. [x] update all related docs (API, README, inline, etc.), if any 3. [x] draft PR with a title `(): TASK-1234` 4. [x] tag PR: at least `frontend` or `backend` unless it's global 5. [x] fill in the template below and delete template comments 6. [x] review thyself: read the diff and repro the preview as written 7. [x] open PR & confirm that CI passes 8. [x] request reviewers, if needed 9. [ ] delete this section before merging ### đŸ“Ŗ Summary Activity logs is no longer hidden behind a feature flag and i snow accessible via Project > Settings > Activity ### 📖 Description - The feature flag was removed from the enum - The link to the route is now always visible ### 👀 Preview steps 1. ℹī¸ have an account and a project 2. Remove or disable the activity logs feature flag on with: `?ff_activityLogsEnabled=false` if needed 4. Navigate to Project > Settings 5. đŸŸĸ The `Activity` session should be accessible --- jsapp/js/components/formViewSideTabs.es6 | 3 --- jsapp/js/featureFlags.ts | 1 - 2 files changed, 4 deletions(-) diff --git a/jsapp/js/components/formViewSideTabs.es6 b/jsapp/js/components/formViewSideTabs.es6 index d4c693b6ec..8b7aaef55d 100644 --- a/jsapp/js/components/formViewSideTabs.es6 +++ b/jsapp/js/components/formViewSideTabs.es6 @@ -83,8 +83,6 @@ class FormViewSideTabs extends Reflux.Component { renderFormSideTabs() { var sideTabs = []; - const isActivityLogsEnabled = checkFeatureFlag(FeatureFlag.activityLogsEnabled); - if ( this.state.asset && this.state.asset.has_deployment && @@ -167,7 +165,6 @@ class FormViewSideTabs extends Reflux.Component { } if ( - isActivityLogsEnabled && userCan( PERMISSIONS_CODENAMES.manage_asset, this.state.asset diff --git a/jsapp/js/featureFlags.ts b/jsapp/js/featureFlags.ts index ccc1d98890..14acbd112d 100644 --- a/jsapp/js/featureFlags.ts +++ b/jsapp/js/featureFlags.ts @@ -3,7 +3,6 @@ * For our sanity, use camel case and match key with value. */ export enum FeatureFlag { - activityLogsEnabled = 'activityLogsEnabled', mmosEnabled = 'mmosEnabled', oneTimeAddonsEnabled = 'oneTimeAddonsEnabled', exportActivityLogsEnabled = 'exportActivityLogsEnabled', From 0adae37739984aa12ffc753e98723c49d9bd69ab Mon Sep 17 00:00:00 2001 From: James Kiger <68701146+jamesrkiger@users.noreply.github.com> Date: Thu, 19 Dec 2024 07:20:03 -0500 Subject: [PATCH 6/6] fix(organizations): restrict my organization projects to admins/owners TASK-1393 (#5384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Fixes viewswitcher so only MMO admins and owners see option for `myOrgProjectsRoute` and restricts route itself based on user's org role. ### 👀 Preview steps 1. Create/use an MMO with at least three members: owner, admin and member 2. Login as each member and view projects list. 3. Observe that project list view switcher only gives "my org projects" option for admin and owner 4. Observe that member is redirected to normal project route when trying to navigate to org project route directly via URL --- .../js/projects/projectViews/viewSwitcher.tsx | 24 ++++++++++++------- jsapp/js/projects/routes.tsx | 5 ++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/jsapp/js/projects/projectViews/viewSwitcher.tsx b/jsapp/js/projects/projectViews/viewSwitcher.tsx index 143e1c6313..0c56a34c43 100644 --- a/jsapp/js/projects/projectViews/viewSwitcher.tsx +++ b/jsapp/js/projects/projectViews/viewSwitcher.tsx @@ -10,7 +10,10 @@ import KoboDropdown from 'js/components/common/koboDropdown'; // Stores and hooks import projectViewsStore from './projectViewsStore'; -import {useOrganizationQuery} from 'js/account/organization/organizationQuery'; +import { + useOrganizationQuery, + OrganizationUserRole, +} from 'js/account/organization/organizationQuery'; // Constants import {PROJECTS_ROUTES} from 'js/router/routerConstants'; @@ -49,10 +52,15 @@ function ViewSwitcher(props: ViewSwitcherProps) { } }; - const hasMultipleOptions = ( - projectViews.views.length !== 0 || - orgQuery.data?.is_mmo - ); + const displayMyOrgOption = + orgQuery.data?.is_mmo && + [OrganizationUserRole.admin, OrganizationUserRole.owner].includes( + orgQuery.data?.request_user_role + ); + + const hasMultipleOptions = + projectViews.views.length !== 0 || displayMyOrgOption; + const organizationName = orgQuery.data?.name || t('Organization'); let triggerLabel = HOME_VIEW.name; @@ -109,9 +117,9 @@ function ViewSwitcher(props: ViewSwitcherProps) { {HOME_VIEW.name} </button> - {/* This is the organization view option - depends if user is in MMO - organization */} - {orgQuery.data?.is_mmo && + {/* This is the organization view option - restricted to + MMO admins and owners */} + {displayMyOrgOption && <button key={ORG_VIEW.uid} className={styles.menuOption} diff --git a/jsapp/js/projects/routes.tsx b/jsapp/js/projects/routes.tsx index 3076166d1c..7c88f041af 100644 --- a/jsapp/js/projects/routes.tsx +++ b/jsapp/js/projects/routes.tsx @@ -3,6 +3,7 @@ import {Navigate, Route} from 'react-router-dom'; import RequireAuth from 'js/router/requireAuth'; import {PROJECTS_ROUTES} from 'js/router/routerConstants'; import {RequireOrgPermissions} from 'js/router/RequireOrgPermissions.component'; +import { OrganizationUserRole } from '../account/organization/organizationQuery'; const MyProjectsRoute = React.lazy( () => import(/* webpackPrefetch: true */ './myProjectsRoute') @@ -34,6 +35,10 @@ export default function routes() { element={ <RequireAuth> <RequireOrgPermissions + validRoles={[ + OrganizationUserRole.owner, + OrganizationUserRole.admin, + ]} mmoOnly redirectRoute={PROJECTS_ROUTES.MY_PROJECTS} >