Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto org selection with cross-org object switcher #5823

Merged
merged 9 commits into from
Jan 24, 2025
Merged
2 changes: 1 addition & 1 deletion temba/api/internal/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from temba.tests import TembaTest, matchers
from temba.tickets.models import Shortcut, TicketExport

NUM_BASE_QUERIES = 4 # number of queries required for any request (internal API is session only)
NUM_BASE_QUERIES = 3 # number of queries required for any request (internal API is session only)


class EndpointsTest(APITestMixin, TembaTest):
Expand Down
2 changes: 1 addition & 1 deletion temba/api/v2/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@


class APITest(APITestMixin, TembaTest):
BASE_SESSION_QUERIES = 4 # number of queries required for any request using session auth
BASE_SESSION_QUERIES = 3 # number of queries required for any request using session auth
BASE_TOKEN_QUERIES = 2 # number of queries required for any request using token auth

def upload_media(self, user, filename: str):
Expand Down
6 changes: 3 additions & 3 deletions temba/channels/types/rocketchat/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,15 @@ def new_form_data(self, path=None, scheme=None) -> dict:
def submit_form(self, data, mock_generate_secret, mock_socket):
mock_generate_secret.return_value = self.secret

self.client.force_login(self.admin)
self.login(self.admin)

return self.client.post(self.claim_url, data=data)

@patch("temba.channels.types.rocketchat.views.generate_secret")
def test_session_key(self, mock_generate_secret):
mock_generate_secret.return_value = self.secret

self.client.force_login(self.admin)
self.login(self.admin)
response = self.client.get(self.claim_url)
self.assertEqual(response.wsgi_request.session.get(ClaimView.SESSION_KEY), self.secret)
response.wsgi_request.session.pop(ClaimView.SESSION_KEY, None)
Expand All @@ -126,7 +126,7 @@ def test_session_key(self, mock_generate_secret):
def test_form_initial(self, mock_generate_secret):
mock_generate_secret.return_value = self.secret

self.client.force_login(self.admin)
self.login(self.admin)
response = self.client.get(self.claim_url)
self.assertEqual(response.context_data["form"].initial.get("secret"), self.secret)

Expand Down
6 changes: 3 additions & 3 deletions temba/contacts/tests/test_contact.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ def omnibox_request(query: str):
)

# empty query just returns up to 25 groups A-Z
with self.assertNumQueries(10):
with self.assertNumQueries(9):
self.assertEqual(
[
{"id": str(joe_and_frank.uuid), "name": "Joe and Frank", "type": "group", "count": 2},
Expand All @@ -538,7 +538,7 @@ def omnibox_request(query: str):
omnibox_request(""),
)

with self.assertNumQueries(13):
with self.assertNumQueries(12):
mr_mocks.contact_search(query='name ~ "250" OR urn ~ "250"', total=2, contacts=[self.billy, self.frank])

self.assertEqual(
Expand All @@ -549,7 +549,7 @@ def omnibox_request(query: str):
omnibox_request("?search=250"),
)

with self.assertNumQueries(14):
with self.assertNumQueries(13):
mr_mocks.contact_search(query='name ~ "FRA" OR urn ~ "FRA"', total=1, contacts=[self.frank])

self.assertEqual(
Expand Down
10 changes: 5 additions & 5 deletions temba/contacts/tests/test_contactcrudl.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def test_list(self, mr_mocks):

self.login(self.editor)

with self.assertNumQueries(16):
with self.assertNumQueries(15):
response = self.client.get(list_url)

self.assertEqual([frank, joe], list(response.context["object_list"]))
Expand Down Expand Up @@ -455,10 +455,10 @@ def test_group(self, mr_mocks):
response = self.requestView(group3_url, self.admin)
self.assertLoginRedirect(response)

# if the user has access to that org, we redirect to the org choose page
# if the user has access to that org, we redirect to the switch page
self.org2.add_user(self.admin, OrgRole.ADMINISTRATOR)
response = self.requestView(group3_url, self.admin)
self.assertRedirect(response, "/org/choose/")
response = self.requestView(group3_url, self.admin, choose_org=self.org)
self.assertRedirect(response, "/org/switch/")

@mock_mailroom
def test_read(self, mr_mocks):
Expand Down Expand Up @@ -644,7 +644,7 @@ def test_history(self):

# fetch our contact history
self.login(self.admin)
with self.assertNumQueries(25):
with self.assertNumQueries(24):
response = self.client.get(history_url + "?limit=100")

# history should include all messages in the last 90 days, the channel event, the call, and the flow run
Expand Down
2 changes: 1 addition & 1 deletion temba/ivr/tests/test_callcrudl.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_list(self):

# check query count
self.login(self.admin)
with self.assertNumQueries(10):
with self.assertNumQueries(9):
self.client.get(list_url)

self.assertRequestDisallowed(list_url, [None, self.agent])
Expand Down
6 changes: 3 additions & 3 deletions temba/locations/tests/test_location.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def test_boundaries(self):
)

# update our alias for east
with self.assertNumQueries(13):
with self.assertNumQueries(12):
response = self.client.post(
reverse("locations.adminboundary_boundaries", args=[self.country.osm_id]),
json.dumps(dict(osm_id=self.state2.osm_id, aliases="kigs\n")),
Expand All @@ -138,7 +138,7 @@ def test_boundaries(self):
self.assertEqual(200, response.status_code)

# fetch our aliases
with self.assertNumQueries(18):
with self.assertNumQueries(17):
response = self.client.get(reverse("locations.adminboundary_boundaries", args=[self.country.osm_id]))
response_json = response.json()

Expand All @@ -157,7 +157,7 @@ def test_boundaries(self):
self.assertEqual(200, response.status_code)

# fetch our aliases
with self.assertNumQueries(25):
with self.assertNumQueries(24):
response = self.client.get(reverse("locations.adminboundary_boundaries", args=[self.state1.osm_id]))
response_json = response.json()

Expand Down
17 changes: 9 additions & 8 deletions temba/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from django.http import HttpResponseForbidden
from django.utils import timezone, translation

from temba.orgs.models import Org, User
from temba.orgs.models import Org


class ExceptionMiddleware:
Expand Down Expand Up @@ -77,13 +77,14 @@ def determine_org(self, request):
org = Org.objects.filter(is_active=True, id=org_id).select_related(*self.select_related).first()

# only use if user actually belongs to this org
if org and (user.is_staff or org.has_user(user)):
return org

# otherwise if user only belongs to one org, we can use that
user_orgs = User.get_orgs_for_request(request)
if user_orgs.count() == 1:
return user_orgs[0]
if org:
if user.is_staff:
return org

membership = org.get_membership(user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this could be combined with line 77

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it feels pretty clear as written to me, what are you proposing exactly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking line 77 could both filter by org id and user membership but it's fine keeping those separate

if membership:
membership.record_seen()
return org

return None

Expand Down
12 changes: 6 additions & 6 deletions temba/msgs/tests/test_msgcrudl.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_inbox(self):

# check query count
self.login(self.admin)
with self.assertNumQueries(12):
with self.assertNumQueries(11):
self.client.get(inbox_url)

self.assertRequestDisallowed(inbox_url, [None, self.agent])
Expand Down Expand Up @@ -129,7 +129,7 @@ def test_flows(self):

# check query count
self.login(self.admin)
with self.assertNumQueries(12):
with self.assertNumQueries(11):
self.client.get(flows_url)

self.assertRequestDisallowed(flows_url, [None, self.agent])
Expand All @@ -150,7 +150,7 @@ def test_archived(self):

# check query count
self.login(self.admin)
with self.assertNumQueries(12):
with self.assertNumQueries(11):
self.client.get(archived_url)

self.assertRequestDisallowed(archived_url, [None, self.agent])
Expand Down Expand Up @@ -193,7 +193,7 @@ def test_outbox(self):

# check query count
self.login(self.admin)
with self.assertNumQueries(11):
with self.assertNumQueries(10):
self.client.get(outbox_url)

# messages sorted by created_on
Expand Down Expand Up @@ -241,7 +241,7 @@ def test_sent(self):

# check query count
self.login(self.admin)
with self.assertNumQueries(10):
with self.assertNumQueries(9):
self.client.get(sent_url)

# messages sorted by sent_on
Expand Down Expand Up @@ -270,7 +270,7 @@ def test_failed(self, mr_mocks):

# check query count
self.login(self.admin)
with self.assertNumQueries(10):
with self.assertNumQueries(9):
self.client.get(failed_url)

self.assertRequestDisallowed(failed_url, [None, self.agent])
Expand Down
2 changes: 1 addition & 1 deletion temba/orgs/migrations/0165_grant_prometheus_feature.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.db import migrations


def grant_prometheus_feature(apps, schema_editor):
def grant_prometheus_feature(apps, schema_editor): # pragma: no cover
Org = apps.get_model("orgs", "Org")

num_updated = 0
Expand Down
18 changes: 18 additions & 0 deletions temba/orgs/migrations/0166_orgmembership_last_seen_on.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 5.1.4 on 2025-01-23 22:25

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("orgs", "0165_grant_prometheus_feature"),
]

operations = [
migrations.AddField(
model_name="orgmembership",
name="last_seen_on",
field=models.DateTimeField(null=True),
),
]
11 changes: 11 additions & 0 deletions temba/orgs/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import pycountry
import pyotp
import pytz
from django_redis import get_redis_connection
from packaging.version import Version
from smartmin.models import SmartModel
from timezone_field import TimeZoneField
Expand Down Expand Up @@ -1444,11 +1445,21 @@ class OrgMembership(models.Model):
user = models.ForeignKey(User, on_delete=models.CASCADE)
role_code = models.CharField(max_length=1)
team = models.ForeignKey("tickets.Team", on_delete=models.PROTECT, null=True)
last_seen_on = models.DateTimeField(null=True)

@property
def role(self):
return OrgRole.from_code(self.role_code)

def record_seen(self):
r = get_redis_connection()
r.sadd("org_members_seen", self.id)

@classmethod
def get_seen(self) -> list:
r = get_redis_connection()
return [k.decode() for k in r.spop("org_members_seen", r.scard("org_members_seen"))]

class Meta:
unique_together = (("org", "user"),)

Expand Down
12 changes: 11 additions & 1 deletion temba/orgs/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,17 @@
from temba.utils.crons import cron_task
from temba.utils.email import EmailSender

from .models import Export, Invitation, ItemCount, Org, OrgImport, User, UserSettings
from .models import Export, Invitation, ItemCount, Org, OrgImport, OrgMembership, User, UserSettings


@cron_task()
def update_members_seen():
"""
Updates last_seen_on for OrgMemberships. We do this in a task every 60 seconds rather than on every request
"""
membership_ids = OrgMembership.get_seen()
if membership_ids:
OrgMembership.objects.filter(id__in=membership_ids).update(last_seen_on=timezone.now())


@shared_task
Expand Down
10 changes: 0 additions & 10 deletions temba/orgs/tests/test_migrations.py

This file was deleted.

11 changes: 11 additions & 0 deletions temba/orgs/tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from temba.api.models import APIToken
from temba.orgs.models import OrgRole, User, UserSettings
from temba.orgs.tasks import update_members_seen
from temba.tests import TembaTest, mock_mailroom


Expand Down Expand Up @@ -148,3 +149,13 @@ def test_release(self, mr_mocks):

token.refresh_from_db()
self.assertFalse(token.is_active)

def test_last_seen(self):
membership = self.org.get_membership(self.admin)
membership.record_seen()
self.assertIsNone(membership.last_seen_on)

update_members_seen()

membership.refresh_from_db()
self.assertIsNotNone(membership.last_seen_on)
5 changes: 3 additions & 2 deletions temba/orgs/views/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ def pre_process(self, request, *args, **kwargs):
f"{reverse('staff.org_service')}?next={quote_plus(request.path)}&other_org={org.id}"
)
else:
# TODO implement view to let regular users switch orgs as well
return HttpResponseRedirect(reverse("orgs.org_choose"))
return HttpResponseRedirect(
f"{reverse('orgs.org_switch')}?next={quote_plus(request.path)}&other_org={org.id}"
)

return super().pre_process(request, *args, **kwargs)

Expand Down
18 changes: 0 additions & 18 deletions temba/orgs/views/tests/test_migrations.py

This file was deleted.

Loading
Loading