From 4f5af46dfc5ff62df1ebe283eb38b29063f3e69b Mon Sep 17 00:00:00 2001 From: Leszek Pietrzak Date: Wed, 27 Nov 2024 15:49:25 +0100 Subject: [PATCH 1/9] feat(InlineMessage): add new type TASK-987 (#5305) Add new "info" type to `InlineMessage` (basically a blue color). --- jsapp/js/components/common/inlineMessage.scss | 5 +++++ jsapp/js/components/common/inlineMessage.tsx | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/jsapp/js/components/common/inlineMessage.scss b/jsapp/js/components/common/inlineMessage.scss index 3331b59ecc..1cff670503 100644 --- a/jsapp/js/components/common/inlineMessage.scss +++ b/jsapp/js/components/common/inlineMessage.scss @@ -50,6 +50,11 @@ .k-icon {color: colors.$kobo-amber;} } +.k-inline-message--type-info { + background-color: colors.$kobo-bg-blue; + .k-icon {color: colors.$kobo-blue;} +} + // We need a bit stronger specificity here .k-inline-message p.k-inline-message__message { margin: 0; diff --git a/jsapp/js/components/common/inlineMessage.tsx b/jsapp/js/components/common/inlineMessage.tsx index 75dc919add..fce85fb18e 100644 --- a/jsapp/js/components/common/inlineMessage.tsx +++ b/jsapp/js/components/common/inlineMessage.tsx @@ -5,7 +5,7 @@ import Icon from 'js/components/common/icon'; import './inlineMessage.scss'; /** Influences the background color and the icon color */ -export type InlineMessageType = 'default' | 'error' | 'success' | 'warning'; +export type InlineMessageType = 'default' | 'error' | 'success' | 'warning' | 'info'; interface InlineMessageProps { type: InlineMessageType; From b8184e8fc6ab896da60ec7aceab2d50d49e5639b Mon Sep 17 00:00:00 2001 From: Paulo Amorim Date: Wed, 27 Nov 2024 14:46:19 -0300 Subject: [PATCH 2/9] refactor(organizations): add of useSession hook TASK-1305 (#5303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 👀 Preview steps No behavior will change with this implementation, but it can be tested by using the new hook somewhere. As a test I've modified `MyProjectRoute` to verify that `currentLoggedAccount` was properly dispatching component re-render and that using hooks methods are working properly as well: ```typescript ... export default function MyProjectsRoute() { const [counter, setCounter] = React.useState(0); const {currentLoggedAccount, refreshAccount} = useSession(); useEffect(() => { setCounter((prev) => prev + 1); }, [currentLoggedAccount]); return ( <> { + const [currentLoggedAccount, setCurrentLoggedAccount] = + useState(); + const [isAnonymous, setIsAnonymous] = useState(true); + const [isPending, setIsPending] = useState(false); + + useEffect(() => { + // We need to setup a reaction for every observable we want to track + // Generic reaction to sessionStore won't fire the re-rendering of the hook + const currentAccountReactionDisposer = reaction( + () => sessionStore.currentAccount, + (currentAccount) => { + if (sessionStore.isLoggedIn) { + setCurrentLoggedAccount(currentAccount as AccountResponse); + setIsAnonymous(false); + setIsPending(sessionStore.isPending); + } + }, + {fireImmediately: true} + ); + + return () => { + currentAccountReactionDisposer(); + }; + }, []); + + return { + currentLoggedAccount, + isAnonymous, + isPending, + logOut: sessionStore.logOut.bind(sessionStore), + logOutAll: sessionStore.logOutAll.bind(sessionStore), + refreshAccount: sessionStore.refreshAccount.bind(sessionStore), + }; +}; From 0f1275b018b41c77b99cddd749821f8ed1910d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20L=C3=A9ger?= Date: Wed, 27 Nov 2024 14:04:18 -0500 Subject: [PATCH 3/9] chore(attachmentStorage): Update sync management command to support no lock during execution (#5307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Added an option to the `update_attachment_storage_bytes` management command to prevent user submission suspension. Updated logic to handle the `submissions_suspended` flag conditionally. --- kobo/apps/openrosa/apps/logger/constants.py | 2 + .../update_attachment_storage_bytes.py | 139 +++++++++++------- kobo/apps/openrosa/apps/logger/tasks.py | 89 ++++++++++- kpi/tasks.py | 1 - 4 files changed, 175 insertions(+), 56 deletions(-) create mode 100644 kobo/apps/openrosa/apps/logger/constants.py diff --git a/kobo/apps/openrosa/apps/logger/constants.py b/kobo/apps/openrosa/apps/logger/constants.py new file mode 100644 index 0000000000..fa6d0dd696 --- /dev/null +++ b/kobo/apps/openrosa/apps/logger/constants.py @@ -0,0 +1,2 @@ + +SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY = 'kobo:update_attachment_storage_bytes:heartbeat' diff --git a/kobo/apps/openrosa/apps/logger/management/commands/update_attachment_storage_bytes.py b/kobo/apps/openrosa/apps/logger/management/commands/update_attachment_storage_bytes.py index 0f6e328964..470fb07368 100644 --- a/kobo/apps/openrosa/apps/logger/management/commands/update_attachment_storage_bytes.py +++ b/kobo/apps/openrosa/apps/logger/management/commands/update_attachment_storage_bytes.py @@ -1,10 +1,16 @@ from __future__ import annotations +import time + from django.conf import settings from django.contrib.auth import get_user_model from django.core.management.base import BaseCommand from django.db.models import Sum, OuterRef, Subquery +from django_redis import get_redis_connection +from kobo.apps.openrosa.apps.logger.constants import ( + SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY +) from kobo.apps.openrosa.apps.logger.models.attachment import Attachment from kobo.apps.openrosa.apps.logger.models.xform import XForm from kobo.apps.openrosa.apps.main.models.user_profile import UserProfile @@ -12,6 +18,7 @@ class Command(BaseCommand): + help = ( 'Retroactively calculate the total attachment file storage ' 'per xform and user profile' @@ -22,6 +29,7 @@ def __init__(self, *args, **kwargs): self._verbosity = 0 self._force = False self._sync = False + self._redis_client = get_redis_connection() def add_arguments(self, parser): parser.add_argument( @@ -52,10 +60,14 @@ def add_arguments(self, parser): ) parser.add_argument( - '-l', '--skip-lock-release', + '-nl', '--no-lock', action='store_true', default=False, - help='Do not attempts to remove submission lock on user profiles. Default is False', + help=( + 'Do not lock accounts from receiving submissions while updating ' + 'storage counters.\n' + 'WARNING: This may result in discrepancies. The default value is False.' + ) ) def handle(self, *args, **kwargs): @@ -65,7 +77,7 @@ def handle(self, *args, **kwargs): self._sync = kwargs['sync'] chunks = kwargs['chunks'] username = kwargs['username'] - skip_lock_release = kwargs['skip_lock_release'] + no_lock = kwargs['no_lock'] if self._force and self._sync: self.stderr.write( @@ -89,7 +101,7 @@ def handle(self, *args, **kwargs): '`force` option has been enabled' ) - if not skip_lock_release: + if not no_lock: self._release_locks() profile_queryset = self._reset_user_profile_counters() @@ -112,57 +124,62 @@ def handle(self, *args, **kwargs): ) continue - self._lock_user_profile(user) + if not no_lock: + self._lock_user_profile(user) - for xform in user_xforms.iterator(chunk_size=chunks): + try: + for xform in user_xforms.iterator(chunk_size=chunks): - # write out xform progress - if self._verbosity > 1: - self.stdout.write( - f"Calculating attachments for xform_id #{xform['pk']}" - f" (user {user.username})" - ) - # aggregate total media file size for all media per xform - form_attachments = Attachment.objects.filter( - instance__xform_id=xform['pk'], - ).aggregate(total=Sum('media_file_size')) - - if form_attachments['total']: - if ( - xform['attachment_storage_bytes'] - == form_attachments['total'] - ): - if self._verbosity > 2: - self.stdout.write( - '\tSkipping xform update! ' - 'Attachment storage is already accurate' + self._heartbeat(user) + + # write out xform progress + if self._verbosity > 1: + self.stdout.write( + f"Calculating attachments for xform_id #{xform['pk']}" + f" (user {user.username})" + ) + # aggregate total media file size for all media per xform + form_attachments = Attachment.objects.filter( + instance__xform_id=xform['pk'], + ).aggregate(total=Sum('media_file_size')) + + if form_attachments['total']: + if ( + xform['attachment_storage_bytes'] + == form_attachments['total'] + ): + if self._verbosity > 2: + self.stdout.write( + '\tSkipping xform update! ' + 'Attachment storage is already accurate' + ) + else: + if self._verbosity > 2: + self.stdout.write( + f'\tUpdating xform attachment storage to ' + f"{form_attachments['total']} bytes" + ) + + XForm.all_objects.filter( + pk=xform['pk'] + ).update( + attachment_storage_bytes=form_attachments['total'] ) + else: if self._verbosity > 2: - self.stdout.write( - f'\tUpdating xform attachment storage to ' - f"{form_attachments['total']} bytes" + self.stdout.write('\tNo attachments found') + if not xform['attachment_storage_bytes'] == 0: + XForm.all_objects.filter( + pk=xform['pk'] + ).update( + attachment_storage_bytes=0 ) - XForm.all_objects.filter( - pk=xform['pk'] - ).update( - attachment_storage_bytes=form_attachments['total'] - ) - - else: - if self._verbosity > 2: - self.stdout.write('\tNo attachments found') - if not xform['attachment_storage_bytes'] == 0: - XForm.all_objects.filter( - pk=xform['pk'] - ).update( - attachment_storage_bytes=0 - ) - - # need to call `update_user_profile()` one more time outside the loop - # because the last user profile will not be up-to-date otherwise - self._update_user_profile(user) + self._update_user_profile(user) + finally: + if not no_lock: + self._release_lock(user) if self._verbosity >= 1: self.stdout.write('Done!') @@ -187,6 +204,13 @@ def _get_queryset(self, profile_queryset, username): return users.order_by('pk') + def _heartbeat(self, user: settings.AUTH_USER_MODEL): + self._redis_client.hset( + SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY, mapping={ + user.username: int(time.time()) + } + ) + def _lock_user_profile(self, user: settings.AUTH_USER_MODEL): # Retrieve or create user's profile. ( @@ -204,8 +228,24 @@ def _lock_user_profile(self, user: settings.AUTH_USER_MODEL): # new submissions from coming in while the # `attachment_storage_bytes` is being calculated. user_profile.metadata['submissions_suspended'] = True + user_profile.metadata['attachments_counting_status'] = 'not-completed' user_profile.save(update_fields=['metadata']) + self._heartbeat(user) + + def _release_lock(self, user: settings.AUTH_USER_MODEL): + # Release any locks on the users' profile from getting submissions + if self._verbosity > 1: + self.stdout.write(f'Releasing submission lock for {user.username}â€Ļ') + + UserProfile.objects.filter(user_id=user.pk).update( + metadata=ReplaceValues( + 'metadata', + updates={'submissions_suspended': False}, + ), + ) + self._redis_client.hdel(SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY, user.username) + def _release_locks(self): # Release any locks on the users' profile from getting submissions if self._verbosity > 1: @@ -249,9 +289,8 @@ def _update_user_profile(self, user: settings.AUTH_USER_MODEL): f'{user.username}’s profile' ) - # Update user's profile (and lock the related row) + # Update user's profile updates = { - 'submissions_suspended': False, 'attachments_counting_status': 'complete', } diff --git a/kobo/apps/openrosa/apps/logger/tasks.py b/kobo/apps/openrosa/apps/logger/tasks.py index a47db21eae..75df1beea3 100644 --- a/kobo/apps/openrosa/apps/logger/tasks.py +++ b/kobo/apps/openrosa/apps/logger/tasks.py @@ -1,6 +1,7 @@ -# coding: utf-8 import csv import datetime +import logging +import time import zipfile from collections import defaultdict from datetime import timedelta @@ -11,14 +12,19 @@ from django.conf import settings from django.core.management import call_command from django.utils import timezone +from django_redis import get_redis_connection from kobo.apps.kobo_auth.shortcuts import User +from kobo.apps.openrosa.libs.utils.jsonbfield_helper import ReplaceValues from kobo.celery import celery_app from kpi.deployment_backends.kc_access.storage import ( default_kobocat_storage as default_storage, ) +from kpi.utils.log import logging +from .constants import SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY from .models.daily_xform_submission_counter import DailyXFormSubmissionCounter from .models import Instance, XForm +from ..main.models import UserProfile @celery_app.task() @@ -46,7 +52,10 @@ def fix_root_node_names(**kwargs): # #### END ISSUE 242 FIX ###### -@shared_task(soft_time_limit=3600, time_limit=3630) +@shared_task( + soft_time_limit=settings.CELERY_LONG_RUNNING_TASK_SOFT_TIME_LIMIT, + time_limit=settings.CELERY_LONG_RUNNING_TASK_TIME_LIMIT +) def generate_stats_zip(output_filename): # Limit to last month and this month now = datetime.datetime.now() @@ -121,6 +130,76 @@ def list_created_by_month(model, date_field): zip_file.close() -@celery_app.task() -def sync_storage_counters(): - call_command('update_attachment_storage_bytes', verbosity=3, sync=True) +@celery_app.task +def fix_stale_submissions_suspended_flag(): + """ + Task to fix stale `submissions_suspended` flag to ensure that accounts are + not indefinitely locked, preventing users from accessing or collecting their + data. + + Note: + - This task is **not** automatically included in the periodic tasks. + - If the task `sync_storage_counters` is added to the periodic tasks, + this task should also be manually added to ensure consistency + in the system's storage management and cleanup process. + """ + + redis_client = get_redis_connection() + lock = redis_client.hgetall(SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY) + if not lock: + return + + usernames = [] + + for username, timestamp in lock.items(): + username = username.decode() + timestamp = int(timestamp.decode()) + + if timestamp + settings.CELERY_LONG_RUNNING_TASK_SOFT_TIME_LIMIT <= int( + time.time() + ): + logging.info( + f'Removing `submission_suspended` flag on user #{username}’s profile' + ) + usernames.append(username) + + if usernames: + UserProfile.objects.filter(user__username__in=usernames).update( + metadata=ReplaceValues( + 'metadata', + updates={'submissions_suspended': False}, + ), + ) + redis_client.hdel(SUBMISSIONS_SUSPENDED_HEARTBEAT_KEY, *usernames) + + +@celery_app.task( + soft_time_limit=settings.CELERY_LONG_RUNNING_TASK_SOFT_TIME_LIMIT, + time_limit=settings.CELERY_LONG_RUNNING_TASK_TIME_LIMIT +) +def sync_storage_counters(**kwargs): + """ + Task to synchronize the "storage" counters for user profiles and their projects (XForm). + + This task ensures consistency between the storage usage tracked at the profile level + and the cumulative storage used by all associated projects. The total storage usage + calculated from the projects should match the storage counter of the corresponding profile. + + Note: + - This task is **not** automatically included in the periodic tasks. + - If this task is added to periodic tasks, ensure that the + `fix_stale_submissions_suspended_flag` task is also scheduled to maintain + system integrity and prevent stale data issues. + """ + + # The `no_lock` option is not hard-coded when calling the command, allowing + # superusers to control the lock behaviour from the admin interface without + # requiring a redeployment. + no_lock = kwargs.get('no_lock', False) + + call_command( + 'update_attachment_storage_bytes', + verbosity=3, + sync=True, + no_lock=no_lock, + ) diff --git a/kpi/tasks.py b/kpi/tasks.py index cfbe91fb52..786484fec8 100644 --- a/kpi/tasks.py +++ b/kpi/tasks.py @@ -93,7 +93,6 @@ def enketo_flush_cached_preview(server_url, form_id): response.raise_for_status() - @celery_app.task(time_limit=LIMIT_HOURS_23, soft_time_limit=LIMIT_HOURS_23) def perform_maintenance(): """ From 8f19a41925104c11e132b758c6ef0c839630249a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20L=C3=A9ger?= Date: Wed, 27 Nov 2024 14:15:36 -0500 Subject: [PATCH 4/9] fix(projectOwnership): set new owner's usage storage correctly (#5308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Fixed an issue where the usage storage for the new owner was not updated correctly during project ownership transfers. --- kobo/apps/openrosa/apps/logger/signals.py | 6 ++---- kobo/apps/openrosa/libs/utils/logger_tools.py | 7 ++++++- kpi/deployment_backends/kobocat_backend.py | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/kobo/apps/openrosa/apps/logger/signals.py b/kobo/apps/openrosa/apps/logger/signals.py index 760e407205..1c67880861 100644 --- a/kobo/apps/openrosa/apps/logger/signals.py +++ b/kobo/apps/openrosa/apps/logger/signals.py @@ -54,10 +54,8 @@ def pre_delete_attachment(instance, **kwargs): if file_size and attachment.deleted_at is None: with transaction.atomic(): - """ - Update both counters at the same time (in a transaction) to avoid - desynchronization as much as possible - """ + # Update both counters simultaneously within a transaction to minimize + # the risk of desynchronization. UserProfile.objects.filter( user_id=xform.user_id ).update( diff --git a/kobo/apps/openrosa/libs/utils/logger_tools.py b/kobo/apps/openrosa/libs/utils/logger_tools.py index e0cc654973..d92ed321fb 100644 --- a/kobo/apps/openrosa/libs/utils/logger_tools.py +++ b/kobo/apps/openrosa/libs/utils/logger_tools.py @@ -734,7 +734,6 @@ def get_soft_deleted_attachments(instance: Instance) -> list[Attachment]: # Update Attachment objects to hide them if they are not used anymore. # We do not want to delete them until the instance itself is deleted. - # FIXME Temporary hack to leave background-audio files and audit files alone # Bug comes from `get_xform_media_question_xpaths()` queryset = Attachment.objects.filter(instance=instance).exclude( @@ -744,6 +743,12 @@ def get_soft_deleted_attachments(instance: Instance) -> list[Attachment]: | Q(media_file_basename__regex=r'^\d{10,}\.(m4a|amr)$') ) soft_deleted_attachments = list(queryset.all()) + + # The query below updates only the database records, not the in-memory + # `Attachment` objects. + # As a result, the `deleted_at` attribute of `Attachment` objects remains `None` + # in memory after the update. + # This behavior is necessary to allow the signal to handle file deletion from storage. queryset.update(deleted_at=dj_timezone.now()) return soft_deleted_attachments diff --git a/kpi/deployment_backends/kobocat_backend.py b/kpi/deployment_backends/kobocat_backend.py index a9b0984457..9c10eb3547 100644 --- a/kpi/deployment_backends/kobocat_backend.py +++ b/kpi/deployment_backends/kobocat_backend.py @@ -1325,7 +1325,7 @@ def transfer_counters_ownership(self, new_owner: 'kobo_auth.User'): attachment_storage_bytes=F('attachment_storage_bytes') - self.xform.attachment_storage_bytes ) - KobocatUserProfile.objects.filter(user_id=self.asset.owner.pk).update( + KobocatUserProfile.objects.filter(user_id=new_owner.pk).update( attachment_storage_bytes=F('attachment_storage_bytes') + self.xform.attachment_storage_bytes ) From 511f38e1b18d1ed8fc502cf485eaadb42af9d0ec Mon Sep 17 00:00:00 2001 From: Leszek Pietrzak Date: Thu, 28 Nov 2024 11:52:04 +0100 Subject: [PATCH 5/9] refactor(subscriptionStore): move planName TASK-987 (#5306) --- jsapp/js/account/subscriptionStore.ts | 19 ++++++++++++++++++- jsapp/js/account/usage/yourPlan.component.tsx | 13 +------------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/jsapp/js/account/subscriptionStore.ts b/jsapp/js/account/subscriptionStore.ts index ac9de4411c..e41289dea8 100644 --- a/jsapp/js/account/subscriptionStore.ts +++ b/jsapp/js/account/subscriptionStore.ts @@ -2,7 +2,8 @@ import {makeAutoObservable} from 'mobx'; import {handleApiFail, fetchGet} from 'js/api'; import {ACTIVE_STRIPE_STATUSES, ROOT_URL} from 'js/constants'; import type {PaginatedResponse} from 'js/dataInterface'; -import type {Product, SubscriptionInfo} from 'js/account/stripe.types'; +import {PlanNames, type Product, type SubscriptionInfo} from 'js/account/stripe.types'; +import envStore from 'js/envStore'; const PRODUCTS_URL = '/api/v2/stripe/products/'; @@ -43,6 +44,22 @@ class SubscriptionStore { }); } + /* + * The plan name displayed to the user. This will display, in order of precedence: + * * The user's active plan subscription + * * The FREE_TIER_DISPLAY["name"] setting (if the user registered before FREE_TIER_CUTOFF_DATE + * * The free plan + */ + public get planName() { + if ( + this.planResponse.length && + this.planResponse[0].items.length + ) { + return this.planResponse[0].items[0].price.product.name; + } + return envStore.data?.free_tier_display?.name || PlanNames.FREE; + } + private onFetchSubscriptionInfoDone( response: PaginatedResponse ) { diff --git a/jsapp/js/account/usage/yourPlan.component.tsx b/jsapp/js/account/usage/yourPlan.component.tsx index 665c20263f..1decccae2d 100644 --- a/jsapp/js/account/usage/yourPlan.component.tsx +++ b/jsapp/js/account/usage/yourPlan.component.tsx @@ -33,18 +33,7 @@ export const YourPlan = () => { const [session] = useState(() => sessionStore); const [productsContext] = useContext(ProductsContext); - /* - * The plan name displayed to the user. This will display, in order of precedence: - * * The user's active plan subscription - * * The FREE_TIER_DISPLAY["name"] setting (if the user registered before FREE_TIER_CUTOFF_DATE - * * The free plan - */ - const planName = useMemo(() => { - if (subscriptions.planResponse.length) { - return subscriptions.planResponse[0].items[0].price.product.name; - } - return env.data?.free_tier_display?.name || PlanNames.FREE; - }, [env.isReady, subscriptions.isInitialised]); + const planName = subscriptions.planName; // The start date of the user's plan. Defaults to the account creation date if the user doesn't have a subscription. const startDate = useMemo(() => { From 38393e0bc855663f7b38f7c821f9b831f7d7954b Mon Sep 17 00:00:00 2001 From: James Kiger <68701146+jamesrkiger@users.noreply.github.com> Date: Thu, 28 Nov 2024 09:39:00 -0500 Subject: [PATCH 6/9] feat(organizations): update is_mmo to identify mmo subscriptions TASK-1231 (#5289) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### đŸ“Ŗ Summary Change the logic of the `is_mmo` property on the Organization model to check Stripe product metadata for the `'mmo_enabled'` field. --- kobo/apps/organizations/models.py | 12 ++++- .../tests/test_organizations_api.py | 9 ++-- kobo/apps/organizations/views.py | 7 --- .../stripe/tests/test_organization_usage.py | 49 +++++++++++++------ kobo/apps/stripe/tests/utils.py | 22 ++++----- kpi/tests/test_usage_calculator.py | 4 +- 6 files changed, 60 insertions(+), 43 deletions(-) diff --git a/kobo/apps/organizations/models.py b/kobo/apps/organizations/models.py index a51d2662af..fedff3754a 100644 --- a/kobo/apps/organizations/models.py +++ b/kobo/apps/organizations/models.py @@ -175,11 +175,19 @@ def is_mmo(self): This returns True if: - A superuser has enabled the override (`mmo_override`), or - - The organization has an active subscription. + - The organization has an active subscription to a plan with + mmo_enabled set to 'true' in the Stripe product metadata. If the override is enabled, it takes precedence over the subscription status """ - return self.mmo_override or bool(self.active_subscription_billing_details()) + if self.mmo_override: + return True + + if billing_details := self.active_subscription_billing_details(): + if product_metadata := billing_details.get('product_metadata'): + return product_metadata.get('mmo_enabled') == 'true' + + return False @cache_for_request def is_admin_only(self, user: 'User') -> bool: diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index a3fa635543..5648ca921e 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -34,6 +34,7 @@ class OrganizationApiTestCase(BaseTestCase): 'current_period_start': '2024-01-01', 'current_period_end': '2024-12-31' } + MMO_SUBSCRIPTION_DETAILS = {'product_metadata': {'mmo_enabled': 'true'}} def setUp(self): self.user = User.objects.get(username='someuser') @@ -121,13 +122,13 @@ def test_api_response_includes_is_mmo_with_mmo_override(self): @patch.object( Organization, 'active_subscription_billing_details', - return_value=DEFAULT_SUBSCRIPTION_DETAILS + return_value=MMO_SUBSCRIPTION_DETAILS, ) def test_api_response_includes_is_mmo_with_subscription( self, mock_active_subscription ): """ - Test that is_mmo is True when there is an active subscription. + Test that is_mmo is True when there is an active MMO subscription. """ self._insert_data(mmo_override=False) response = self.client.get(self.url_detail) @@ -154,14 +155,14 @@ def test_api_response_includes_is_mmo_with_no_override_and_no_subscription( @patch.object( Organization, 'active_subscription_billing_details', - return_value=DEFAULT_SUBSCRIPTION_DETAILS + return_value=MMO_SUBSCRIPTION_DETAILS, ) def test_api_response_includes_is_mmo_with_override_and_subscription( self, mock_active_subscription ): """ Test that is_mmo is True when both mmo_override and active - subscription is present. + MMO subscription is present. """ self._insert_data(mmo_override=True) response = self.client.get(self.url_detail) diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index 7fd0e59470..9c3d4634ee 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -9,10 +9,7 @@ OuterRef, ) from django.db.models.expressions import Exists -from django.utils.decorators import method_decorator from django.utils.http import http_date -from django.views.decorators.cache import cache_page -from django_dont_vary_on.decorators import only_vary_on from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.request import Request @@ -69,10 +66,6 @@ def get_queryset(self, *args, **kwargs): raise NotImplementedError -@method_decorator(cache_page(settings.ENDPOINT_CACHE_DURATION), name='service_usage') -# django uses the Vary header in its caching, and each middleware can potentially add more Vary headers -# we use this decorator to remove any Vary headers except 'origin' (we don't want to cache between different installs) -@method_decorator(only_vary_on('Origin'), name='service_usage') class OrganizationViewSet(viewsets.ModelViewSet): """ Organizations are groups of users with assigned permissions and configurations diff --git a/kobo/apps/stripe/tests/test_organization_usage.py b/kobo/apps/stripe/tests/test_organization_usage.py index 788122553e..a349361bff 100644 --- a/kobo/apps/stripe/tests/test_organization_usage.py +++ b/kobo/apps/stripe/tests/test_organization_usage.py @@ -18,7 +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_enterprise_subscription, + generate_mmo_subscription, generate_plan_subscription, ) from kobo.apps.stripe.utils import get_organization_plan_limit @@ -102,7 +102,7 @@ def test_usage_for_plans_with_org_access(self): when viewing /service_usage/{organization_id}/ """ - generate_enterprise_subscription(self.organization) + generate_mmo_subscription(self.organization) # the user should see usage for everyone in their org response = self.client.get(self.detail_url) @@ -133,7 +133,7 @@ def test_endpoint_speed(self): # get the average request time for 10 hits to the endpoint single_user_time = timeit.timeit(lambda: self.client.get(self.detail_url), number=10) - generate_enterprise_subscription(self.organization) + generate_mmo_subscription(self.organization) # get the average request time for 10 hits to the endpoint multi_user_time = timeit.timeit(lambda: self.client.get(self.detail_url), number=10) @@ -146,7 +146,7 @@ def test_endpoint_is_cached(self): """ Test that multiple hits to the endpoint from the same origin are properly cached """ - generate_enterprise_subscription(self.organization) + generate_mmo_subscription(self.organization) first_response = self.client.get(self.detail_url) assert first_response.data['total_submission_count']['current_month'] == self.expected_submissions_multi @@ -433,7 +433,7 @@ def test_user_not_member_of_organization(self): assert response.status_code == status.HTTP_400_BAD_REQUEST def test_successful_retrieval(self): - generate_enterprise_subscription(self.organization) + generate_mmo_subscription(self.organization) create_mock_assets([self.anotheruser]) response = self.client.get(self.detail_url) assert response.status_code == status.HTTP_200_OK @@ -441,8 +441,8 @@ def test_successful_retrieval(self): assert response.data['results'][0]['asset__name'] == 'test' assert response.data['results'][0]['deployment_status'] == 'deployed' - def test_aggregates_usage_for_enterprise_org(self): - generate_enterprise_subscription(self.organization) + def test_aggregates_usage_for_mmo(self): + generate_mmo_subscription(self.organization) self.organization.add_user(self.newuser) # create 2 additional assets, one per user create_mock_assets([self.anotheruser, self.newuser]) @@ -450,14 +450,6 @@ def test_aggregates_usage_for_enterprise_org(self): assert response.status_code == status.HTTP_200_OK assert response.data['count'] == 2 - def test_users_without_enterprise_see_only_their_usage(self): - generate_plan_subscription(self.organization) - self.organization.add_user(self.newuser) - create_mock_assets([self.anotheruser, self.newuser]) - response = self.client.get(self.detail_url) - assert response.status_code == status.HTTP_200_OK - assert response.data['count'] == 1 - @ddt class OrganizationsUtilsTestCase(BaseTestCase): @@ -473,7 +465,7 @@ def setUp(self): self.organization.add_user(self.anotheruser, is_admin=True) def test_get_plan_community_limit(self): - generate_enterprise_subscription(self.organization) + generate_mmo_subscription(self.organization) limit = get_organization_plan_limit(self.organization, 'seconds') assert limit == 2000 # TODO get the limits from the community plan, overrides limit = get_organization_plan_limit(self.organization, 'characters') @@ -504,3 +496,28 @@ def test_get_suscription_limit_unlimited(self, usage_type): generate_plan_subscription(self.organization, metadata=product_metadata) limit = get_organization_plan_limit(self.organization, usage_type) assert limit == float('inf') + + +@override_settings(STRIPE_ENABLED=True) +class OrganizationsModelIntegrationTestCase(BaseTestCase): + fixtures = ['test_data'] + + def setUp(self): + self.someuser = User.objects.get(username='someuser') + self.organization = self.someuser.organization + + def test_is_mmo_subscription_logic(self): + product_metadata = { + 'mmo_enabled': 'false', + } + subscription = generate_plan_subscription( + self.organization, metadata=product_metadata + ) + assert self.organization.is_mmo is False + subscription.status = 'canceled' + subscription.ended_at = timezone.now() + subscription.save() + + product_metadata['mmo_enabled'] = 'true' + generate_plan_subscription(self.organization, metadata=product_metadata) + assert self.organization.is_mmo is True diff --git a/kobo/apps/stripe/tests/utils.py b/kobo/apps/stripe/tests/utils.py index 55bb9e62af..e7dd249d86 100644 --- a/kobo/apps/stripe/tests/utils.py +++ b/kobo/apps/stripe/tests/utils.py @@ -2,7 +2,7 @@ from dateutil.relativedelta import relativedelta from django.utils import timezone -from djstripe.models import Customer, Product, SubscriptionItem, Subscription, Price +from djstripe.models import Customer, Price, Product, Subscription, SubscriptionItem from model_bakery import baker from kobo.apps.organizations.models import Organization @@ -17,7 +17,6 @@ def generate_plan_subscription( ) -> Subscription: """Create a subscription for a product with custom metadata""" created_date = timezone.now() - relativedelta(days=age_days) - price_id = 'price_sfmOFe33rfsfd36685657' if not customer: customer = baker.make(Customer, subscriber=organization, livemode=False) @@ -30,14 +29,12 @@ def generate_plan_subscription( product_metadata = {**product_metadata, **metadata} product = baker.make(Product, active=True, metadata=product_metadata) - if not (price := Price.objects.filter(id=price_id).first()): - price = baker.make( - Price, - active=True, - id=price_id, - recurring={'interval': interval}, - product=product, - ) + price = baker.make( + Price, + active=True, + recurring={'interval': interval}, + product=product, + ) period_offset = relativedelta(weeks=2) @@ -59,5 +56,6 @@ def generate_plan_subscription( ) -def generate_enterprise_subscription(organization: Organization, customer: Customer = None): - return generate_plan_subscription(organization, {'plan_type': 'enterprise'}, customer) +def generate_mmo_subscription(organization: Organization, customer: Customer = None): + product_metadata = {'mmo_enabled': 'true', 'plan_type': 'enterprise'} + return generate_plan_subscription(organization, product_metadata, customer) diff --git a/kpi/tests/test_usage_calculator.py b/kpi/tests/test_usage_calculator.py index e8a86fe7b8..82c3afc6f7 100644 --- a/kpi/tests/test_usage_calculator.py +++ b/kpi/tests/test_usage_calculator.py @@ -12,7 +12,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.organizations.models import Organization from kobo.apps.stripe.constants import USAGE_LIMIT_MAP -from kobo.apps.stripe.tests.utils import generate_enterprise_subscription +from kobo.apps.stripe.tests.utils import generate_mmo_subscription from kobo.apps.trackers.models import NLPUsageCounter from kpi.models import Asset from kpi.tests.base_test_case import BaseAssetTestCase @@ -201,7 +201,7 @@ def test_organization_setup(self): organization = baker.make(Organization, id='org_abcd1234', mmo_override=True) organization.add_user(user=self.anotheruser, is_admin=True) organization.add_user(user=self.someuser, is_admin=True) - generate_enterprise_subscription(organization) + generate_mmo_subscription(organization) calculator = ServiceUsageCalculator(self.someuser, organization) submission_counters = calculator.get_submission_counters() From aaa8aac0e44360f490fa98ab660c12ed63a5ffcc Mon Sep 17 00:00:00 2001 From: Paulo Amorim Date: Thu, 28 Nov 2024 12:51:37 -0300 Subject: [PATCH 7/9] refactor(organizations): code cleanup and patch data cleanup in account settings and tos view TASK-1292 (#5290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 👀 Preview steps 1. ℹī¸ have an account and a project 2. Go into account settings view 3. Change information on some field(s) 4. Save the changes 5. Check in the network tab the call to the `/me` endpoint. The payload should only contain the changed fields. 6. Enable a TOS view by following [this document](https://www.notion.so/kobotoolbox/Setting-up-TOS-1447e515f65480f8ab34f69d42d806f8) 7. In the TOS view, fill in the information and save (you may want to `preserve log` before saving data) 8. Check the network tab. Payload should contain only the changed field. ### 💭 Notes We're refactoring accountDetails and TOS view on the way of implementing some changes on displayed fields: - Removed the "global" observer from mob-x which was causing linting issues in favor of using the `useSession` hook created at https://github.com/kobotoolbox/kpi/pull/5303 - Cleaned up the code - Changed in a way that only changed fields are sent to the `PATCH` request to update data --- jsapp/js/account/account.utils.ts | 41 +--- .../account/accountFieldsEditor.component.tsx | 5 +- jsapp/js/account/accountSettingsRoute.tsx | 181 ++++++++---------- jsapp/js/tos/tosForm.component.tsx | 80 ++++---- 4 files changed, 130 insertions(+), 177 deletions(-) diff --git a/jsapp/js/account/account.utils.ts b/jsapp/js/account/account.utils.ts index 6a242324db..792f0bb6cb 100644 --- a/jsapp/js/account/account.utils.ts +++ b/jsapp/js/account/account.utils.ts @@ -1,6 +1,4 @@ import type {AccountFieldsValues} from './account.constants'; -import envStore from '../envStore'; -import {USER_FIELD_NAMES} from './account.constants'; export function getInitialAccountFieldsValues(): AccountFieldsValues { return { @@ -25,41 +23,6 @@ export function getInitialAccountFieldsValues(): AccountFieldsValues { * For given field values produces an object to use with the `/me` endpoint for * updating the `extra_details`. */ -export function getProfilePatchData(fields: AccountFieldsValues) { - // HACK: dumb down the `output` type here, so TS doesn't have a problem with - // types inside the `forEach` loop below, and the output is compatible with - // functions from `api.ts` file. - const output: {extra_details: {[key: string]: any}} = { - extra_details: getInitialAccountFieldsValues(), - }; - - // To patch correctly with recent changes to the backend, - // ensure that we send empty strings if the field is left blank. - - // We should only overwrite user metadata that the user can see. - // Fields that: - // (a) are enabled in constance - // (b) the frontend knows about - - // Make a list of user metadata fields to include in the patch - const presentMetadataFields = - // Fields enabled in constance - envStore.data - .getUserMetadataFieldNames() - // Intersected with: - .filter( - (fieldName) => - // Fields the frontend knows about - fieldName in USER_FIELD_NAMES - ); - - // Populate the patch with user form input, or empty strings. - presentMetadataFields.forEach((fieldName) => { - output.extra_details[fieldName] = fields[fieldName] || ''; - }); - - // Always include require_auth, defaults to 'false'. - output.extra_details.require_auth = fields.require_auth ? true : false; - - return output; +export function getProfilePatchData(fields: Partial) { + return {extra_details: fields}; } diff --git a/jsapp/js/account/accountFieldsEditor.component.tsx b/jsapp/js/account/accountFieldsEditor.component.tsx index b52a1cdcaf..ce8e5dea81 100644 --- a/jsapp/js/account/accountFieldsEditor.component.tsx +++ b/jsapp/js/account/accountFieldsEditor.component.tsx @@ -42,7 +42,7 @@ interface AccountFieldsEditorProps { * `displayedFields` prop) */ values: AccountFieldsValues; - onChange: (fields: AccountFieldsValues) => void; + onFieldChange: (fieldName: UserFieldName, value: UserFieldValue) => void; } /** @@ -82,8 +82,7 @@ export default function AccountFieldsEditor(props: AccountFieldsEditorProps) { fieldName: UserFieldName, newValue: UserFieldValue ) { - const newValues = {...props.values, [fieldName]: newValue}; - props.onChange(newValues); + props.onFieldChange(fieldName, newValue); } const cleanedUrl = (value: string) => { diff --git a/jsapp/js/account/accountSettingsRoute.tsx b/jsapp/js/account/accountSettingsRoute.tsx index 91e7b6ea69..bdaf7368ce 100644 --- a/jsapp/js/account/accountSettingsRoute.tsx +++ b/jsapp/js/account/accountSettingsRoute.tsx @@ -1,11 +1,9 @@ -import React, {useEffect, useState} from 'react'; +import type React from 'react'; +import {useEffect, useState} from 'react'; import Button from 'js/components/common/button'; import InlineMessage from 'js/components/common/inlineMessage'; -import {observer} from 'mobx-react'; -import type {Form} from 'react-router-dom'; import {unstable_usePrompt as usePrompt} from 'react-router-dom'; import bem, {makeBem} from 'js/bem'; -import sessionStore from 'js/stores/session'; import './accountSettings.scss'; import {notify} from 'js/utils'; import {dataInterface} from '../dataInterface'; @@ -21,6 +19,7 @@ import type { AccountFieldsErrors, } from './account.constants'; import {HELP_ARTICLE_ANON_SUBMISSIONS_URL} from 'js/constants'; +import {useSession} from '../stores/useSession'; bem.AccountSettings = makeBem(null, 'account-settings', 'form'); bem.AccountSettings__left = makeBem(bem.AccountSettings, 'left'); @@ -28,112 +27,85 @@ bem.AccountSettings__right = makeBem(bem.AccountSettings, 'right'); bem.AccountSettings__item = makeBem(bem.FormModal, 'item'); bem.AccountSettings__actions = makeBem(bem.AccountSettings, 'actions'); -interface Form { - isPristine: boolean; - /** - * Whether we have loaded the user metadata values. Used to avoid displaying - * blank form with values coming in a moment later (in visible way). - */ - isUserDataLoaded: boolean; - fields: AccountFieldsValues; - fieldsWithErrors: { - extra_details?: AccountFieldsErrors; - }; -} - -const AccountSettings = observer(() => { - const [form, setForm] = useState
({ - isPristine: true, - isUserDataLoaded: false, - fields: getInitialAccountFieldsValues(), - fieldsWithErrors: {}, - }); +const AccountSettings = () => { + const [isPristine, setIsPristine] = useState(true); + const [fieldErrors, setFieldErrors] = useState({}); + const [formFields, setFormFields] = useState( + getInitialAccountFieldsValues() + ); + const [editedFields, setEditedFields] = useState< + Partial + >({}); + + const {currentLoggedAccount, refreshAccount} = useSession(); useEffect(() => { - if ( - !sessionStore.isPending && - sessionStore.isInitialLoadComplete && - !sessionStore.isInitialRoute - ) { - sessionStore.refreshAccount(); - } - }, []); - useEffect(() => { - const currentAccount = sessionStore.currentAccount; - if ( - !sessionStore.isPending && - sessionStore.isInitialLoadComplete && - 'email' in currentAccount - ) { - setForm({ - ...form, - isUserDataLoaded: true, - fields: { - name: currentAccount.extra_details.name, - organization_type: currentAccount.extra_details.organization_type, - organization: currentAccount.extra_details.organization, - organization_website: - currentAccount.extra_details.organization_website, - sector: currentAccount.extra_details.sector, - gender: currentAccount.extra_details.gender, - bio: currentAccount.extra_details.bio, - city: currentAccount.extra_details.city, - country: currentAccount.extra_details.country, - require_auth: currentAccount.extra_details.require_auth, - twitter: currentAccount.extra_details.twitter, - linkedin: currentAccount.extra_details.linkedin, - instagram: currentAccount.extra_details.instagram, - newsletter_subscription: - currentAccount.extra_details.newsletter_subscription, - }, - fieldsWithErrors: {}, - }); + if (!currentLoggedAccount) { + return; } - }, [sessionStore.isPending]); + + setFormFields({ + name: currentLoggedAccount.extra_details.name, + organization_type: currentLoggedAccount.extra_details.organization_type, + organization: currentLoggedAccount.extra_details.organization, + organization_website: currentLoggedAccount.extra_details.organization_website, + sector: currentLoggedAccount.extra_details.sector, + gender: currentLoggedAccount.extra_details.gender, + bio: currentLoggedAccount.extra_details.bio, + city: currentLoggedAccount.extra_details.city, + country: currentLoggedAccount.extra_details.country, + require_auth: currentLoggedAccount.extra_details.require_auth, + twitter: currentLoggedAccount.extra_details.twitter, + linkedin: currentLoggedAccount.extra_details.linkedin, + instagram: currentLoggedAccount.extra_details.instagram, + newsletter_subscription: + currentLoggedAccount.extra_details.newsletter_subscription, + }); + }, [currentLoggedAccount]); + usePrompt({ - when: !form.isPristine, + when: !isPristine, message: t('You have unsaved changes. Leave settings without saving?'), }); + + const onUpdateComplete = () => { + notify(t('Updated profile successfully')); + setIsPristine(true); + setFieldErrors({}); + }; + + const onUpdateFail = (errors: AccountFieldsErrors) => { + setFieldErrors(errors); + }; + const updateProfile = (e: React.FormEvent) => { e?.preventDefault?.(); // Prevent form submission page reload - const profilePatchData = getProfilePatchData(form.fields); + const patchData = getProfilePatchData(editedFields); dataInterface - .patchProfile(profilePatchData) + .patchProfile(patchData) .done(() => { onUpdateComplete(); + refreshAccount(); }) .fail((...args: any) => { - onUpdateFail(args); + onUpdateFail(args[0].responseJSON); }); }; - const onAccountFieldsEditorChange = (fields: AccountFieldsValues) => { - setForm({ - ...form, - fields: fields, - isPristine: false, - }); - }; - - const onUpdateComplete = () => { - notify(t('Updated profile successfully')); - setForm({ - ...form, - isPristine: true, - fieldsWithErrors: {}, + const onFieldChange = (fieldName: string, value: string | boolean) => { + setFormFields({ + ...formFields, + [fieldName]: value, }); - }; - - const onUpdateFail = (data: any) => { - setForm({ - ...form, - isPristine: false, - fieldsWithErrors: data[0].responseJSON, + setEditedFields({ + ...editedFields, + [fieldName]: value, }); + setIsPristine(false); }; - const accountName = sessionStore.currentAccount.username; + const accountName = currentLoggedAccount?.username || ''; return ( @@ -143,49 +115,56 @@ const AccountSettings = observer(() => { className='account-settings-save' size={'l'} isSubmit - label={t('Save Changes') + (form.isPristine ? '' : ' *')} + label={t('Save Changes') + (isPristine ? '' : ' *')} /> - + - {sessionStore.isInitialLoadComplete && form.isUserDataLoaded && ( + {currentLoggedAccount && ( - {t('You can now control whether to allow anonymous submissions in web forms for each project. Previously, this was an account-wide setting.')} + {t( + 'You can now control whether to allow anonymous submissions in web forms for each project. Previously, this was an account-wide setting.' + )}   - {t('This privacy feature is now a per-project setting. New projects will require authentication by default.')} + {t( + 'This privacy feature is now a per-project setting. New projects will require authentication by default.' + )}   {t('Learn more about these changes here.')} - )} + } className='anonymous-submission-notice' /> )} ); -}); +}; export default AccountSettings; diff --git a/jsapp/js/tos/tosForm.component.tsx b/jsapp/js/tos/tosForm.component.tsx index 931f8295bb..48213c236a 100644 --- a/jsapp/js/tos/tosForm.component.tsx +++ b/jsapp/js/tos/tosForm.component.tsx @@ -1,7 +1,7 @@ -import React, {useState, useEffect} from 'react'; +import type React from 'react'; +import {useState, useEffect} from 'react'; import Button from 'js/components/common/button'; import envStore from 'js/envStore'; -import sessionStore from 'js/stores/session'; import {fetchGet, fetchPatch, fetchPost, handleApiFail} from 'js/api'; import styles from './tosForm.module.scss'; import type {FailResponse, PaginatedResponse} from 'js/dataInterface'; @@ -16,6 +16,7 @@ import type { AccountFieldsErrors, } from 'js/account/account.constants'; import {currentLang, notify} from 'js/utils'; +import {useSession} from '../stores/useSession'; /** A slug for the `sitewide_messages` endpoint */ const TOS_SLUG = 'terms_of_service'; @@ -51,10 +52,13 @@ export default function TOSForm() { const [announcementMessage, setAnnouncementMessage] = useState< string | undefined >(); - const [fields, setFields] = useState( + const [formFields, setFormFields] = useState( getInitialAccountFieldsValues() ); const [fieldsErrors, setFieldsErrors] = useState({}); + const [editedFields, setEditedFields] = useState< + Partial + >({}); const fieldsToShow = envStore.data.getUserMetadataRequiredFieldNames(); if ( @@ -63,6 +67,8 @@ export default function TOSForm() { fieldsToShow.push('newsletter_subscription'); } + const {currentLoggedAccount, logOut} = useSession(); + // Get TOS message from endpoint useEffect(() => { const getTOS = async () => { @@ -104,30 +110,40 @@ export default function TOSForm() { // (including the non-required ones that will be hidden, but passed to the API // so that they will not get erased). useEffect(() => { - if ('email' in sessionStore.currentAccount) { - const data = sessionStore.currentAccount; - setFields({ - name: data.extra_details.name, - organization: data.extra_details.organization, - organization_website: data.extra_details.organization_website, - organization_type: data.extra_details.organization_type, - sector: data.extra_details.sector, - gender: data.extra_details.gender, - bio: data.extra_details.bio, - city: data.extra_details.city, - country: data.extra_details.country, - require_auth: data.extra_details.require_auth, - twitter: data.extra_details.twitter, - linkedin: data.extra_details.linkedin, - instagram: data.extra_details.instagram, - newsletter_subscription: data.extra_details.newsletter_subscription, - }); + if (!currentLoggedAccount) { + return; } - }, [sessionStore.isAuthStateKnown]); - function onAccountFieldsEditorChange(newFields: AccountFieldsValues) { - setFields(newFields); - } + setFormFields({ + name: currentLoggedAccount.extra_details.name, + organization: currentLoggedAccount.extra_details.organization, + organization_website: + currentLoggedAccount.extra_details.organization_website, + organization_type: currentLoggedAccount.extra_details.organization_type, + sector: currentLoggedAccount.extra_details.sector, + gender: currentLoggedAccount.extra_details.gender, + bio: currentLoggedAccount.extra_details.bio, + city: currentLoggedAccount.extra_details.city, + country: currentLoggedAccount.extra_details.country, + require_auth: currentLoggedAccount.extra_details.require_auth, + twitter: currentLoggedAccount.extra_details.twitter, + linkedin: currentLoggedAccount.extra_details.linkedin, + instagram: currentLoggedAccount.extra_details.instagram, + newsletter_subscription: + currentLoggedAccount.extra_details.newsletter_subscription, + }); + }, [currentLoggedAccount]); + + const onFieldChange = (fieldName: string, value: string | boolean) => { + setFormFields({ + ...formFields, + [fieldName]: value, + }); + setEditedFields({ + ...editedFields, + [fieldName]: value, + }); + }; /** * Submitting does two things (with two consecutive API calls): @@ -146,7 +162,7 @@ export default function TOSForm() { // them. if (fieldsToShow.length > 0) { // Get data for the user endpoint - const profilePatchData = getProfilePatchData(fields); + const profilePatchData = getProfilePatchData(editedFields); try { await fetchPatch(ME_ENDPOINT, profilePatchData); @@ -183,16 +199,12 @@ export default function TOSForm() { function leaveForm() { setIsFormPending(true); - sessionStore.logOut(); + logOut(); } // We are waiting for few pieces of data: the message, fields definitions from // environment endpoint and fields data from me endpoint - if ( - !announcementMessage || - !envStore.isReady || - !sessionStore.isAuthStateKnown - ) { + if (!announcementMessage || !envStore.isReady || !currentLoggedAccount) { return ; } @@ -217,8 +229,8 @@ export default function TOSForm() { )} From 70f195faef418990b39a97a5ca9b44a03dc21fc5 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Mon, 2 Dec 2024 07:43:30 -0500 Subject: [PATCH 8/9] fixup!: Update test_signals.py --- kobo/apps/audit_log/tests/test_signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/tests/test_signals.py b/kobo/apps/audit_log/tests/test_signals.py index 289ee8ae88..d3e6b23c0a 100644 --- a/kobo/apps/audit_log/tests/test_signals.py +++ b/kobo/apps/audit_log/tests/test_signals.py @@ -186,7 +186,7 @@ def test_receivers_add_necessary_fields_to_request(self, signal): sender=Asset, instance=self.asset, user=self.user, - codename=PERM_VIEW_ASSET, + codename=PERM_PARTIAL_SUBMISSIONS, perms={PERM_VIEW_ASSET: [{'_submitted_by': 'someuser'}]}, deny=False, ) From 2d350fc4ea6ae4ef5d5f4d1e7444c8c89fb145dd Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Mon, 2 Dec 2024 07:48:15 -0500 Subject: [PATCH 9/9] fixup!: Update models.py --- kobo/apps/audit_log/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/audit_log/models.py b/kobo/apps/audit_log/models.py index ddb6ccc848..cb8e4a7a06 100644 --- a/kobo/apps/audit_log/models.py +++ b/kobo/apps/audit_log/models.py @@ -765,7 +765,7 @@ def handle_anonymous_user_permissions( # remove each permission as it is logged so we can see if there # are any unusual ones left over for combination, action in ANONYMOUS_USER_PERMISSION_ACTIONS.items(): - # ANONYMOUS_USER_PERMIISSION_ACTIONS has tuples as keys + # ANONYMOUS_USER_PERMISSION_ACTIONS has tuples as keys permission, was_added = combination list_to_update = perms_added if was_added else perms_removed if permission in list_to_update: