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

feat: create organizations slug #393

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion lib/atomic/activities.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule Atomic.Activities do
alias Atomic.Activities.Activity
alias Atomic.Activities.ActivityEnrollment
alias Atomic.Activities.Speaker
alias Atomic.Organizations

@doc """
Returns the list of activities.
Expand Down Expand Up @@ -141,7 +142,27 @@ defmodule Atomic.Activities do
end

@doc """
Returns true if the maximum number of enrollments has been reached.
Returns the list of organizations ids that are associated with an activity.

## Examples

iex> get_activity_organizations!(activity)
[19d7c9e5-4212-4f59-a097-28aaa33c2621, ...]

iex> get_activity_organizations!(activity)
** (Ecto.NoResultsError)
"""
def get_activity_organizations!(activity, preloads \\ [:departments]) do
Repo.preload(activity, preloads)
|> Map.get(:departments, [])
|> Enum.map(& &1.organization_id)
|> Enum.map(fn id ->
Organizations.get_organization!(id).slug
end)
end
Comment on lines +155 to +162
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this anymore, since an activity is, for now, associated to only one organization.


@doc """
Verifies the maximum number of enrollments for an activity.

## Examples

Expand Down
27 changes: 26 additions & 1 deletion lib/atomic/organizations.ex
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,31 @@ defmodule Atomic.Organizations do
|> Repo.update()
end

@doc """
Gets an organization by slug.

## Examples
iex> get_organization_by_slug("foo_bar")
%Organization{}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
%Organization{}
%Organization{}

iex> get_organization_by_slug("unknown")
nil

"""
def get_organization_by_slug(slug) when is_binary(slug) do
Repo.get_by(Organization, slug: slug)
end

@doc """
Returns an `%Ecto.Changeset{}` for changing the organization slug.

## Examples
iex> change_organization_slug(organization)
%Ecto.Changeset{data: %Organization{}}
"""
def change_organization_slug(organization, attrs \\ %{}) do
Organization.slug_changeset(organization, attrs)
end

@doc """
Updates an organization card image.

Expand Down Expand Up @@ -261,7 +286,7 @@ defmodule Atomic.Organizations do
nil

"""
def get_role(user_id, organization_id) do
def get_role(user_id, organization_id) when is_binary(user_id) and is_binary(organization_id) do
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Membership
|> where([m], m.user_id == ^user_id and m.organization_id == ^organization_id)
|> Repo.one()
Expand Down
30 changes: 29 additions & 1 deletion lib/atomic/organizations/organization.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule Atomic.Organizations.Organization do
alias Atomic.Organizations.{Announcement, Board, Card, Department, Membership, Partner}
alias Atomic.Uploaders

@required_fields ~w(name description)a
@required_fields ~w(name slug description)a
@optional_fields ~w(card_image logo)a

@derive {
Expand All @@ -26,6 +26,7 @@ defmodule Atomic.Organizations.Organization do
field :description, :string
field :card_image, Uploaders.Card.Type
field :logo, Uploaders.Logo.Type
field :slug, :string

has_many :departments, Department,
on_replace: :delete_if_exists,
Expand Down Expand Up @@ -64,6 +65,7 @@ defmodule Atomic.Organizations.Organization do
|> cast_embed(:card, with: &Card.changeset/2)
|> cast_attachments(attrs, [:card_image, :logo])
|> validate_required(@required_fields)
|> validate_slug()
|> unique_constraint(:name)
end

Expand All @@ -77,4 +79,30 @@ defmodule Atomic.Organizations.Organization do
organization
|> cast_attachments(attrs, [:logo])
end

@doc """
An organization changeset for changing the slug.
It requires the slug to change otherwise an error is added.
"""
def slug_changeset(organization, attrs) do
organization
|> cast(attrs, [:slug])
|> validate_slug()
|> case do
%{changes: %{slug: _}} = changeset -> changeset
%{} = changeset -> add_error(changeset, :slug, "did not change")
end
end

defp validate_slug(changeset) do
changeset
|> validate_required([:slug])
|> validate_format(:slug, ~r/^[a-zA-Z0-9_.]+$/,
Copy link
Member

Choose a reason for hiding this comment

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

Is it a good idea to allow case sensitive slugs?

Copy link
Member

Choose a reason for hiding this comment

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

I agree they should be case insensitive. Meaning "cesium" and "CeSIUM" both stand for the same slug.

message:
gettext("must only contain alphanumeric characters, numbers, underscores and periods")
)
|> validate_length(:slug, min: 3, max: 30)
|> unsafe_validate_unique(:slug, Atomic.Repo)
|> unique_constraint(:slug)
end
end
121 changes: 109 additions & 12 deletions lib/atomic_web/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,28 @@ defmodule AtomicWeb.Config do
@moduledoc """
Web configuration for our pages.
"""

alias Atomic.Accounts
alias Atomic.Organizations
alias AtomicWeb.Router.Helpers, as: Routes

# User is not logged in
def pages(conn, nil, _current_organization) do
default_pages(conn)
def pages(conn, nil, current_organization) do
default_pages(conn, current_organization)
end

# User is logged in
def pages(conn, current_user, current_organization) do

if has_permissions?(current_user, current_organization) do
admin_pages(conn, current_organization)
else
user_pages(conn)
user_pages(conn, current_organization)
end
end

def user_pages(conn) do
default_pages(conn) ++
def user_pages(conn, current_organization) do
default_pages(conn, current_organization) ++
[
%{
key: :scanner,
Expand All @@ -33,34 +36,34 @@ defmodule AtomicWeb.Config do
end

def admin_pages(conn, current_organization) do
default_pages(conn) ++
default_pages(conn, current_organization) ++
[
%{
key: :departments,
title: "Departments",
icon: :cube,
url: Routes.department_index_path(conn, :index, current_organization.id),
url: Routes.department_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :partners,
title: "Partners",
icon: :user_group,
url: Routes.partner_index_path(conn, :index, current_organization.id),
url: Routes.partner_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :memberships,
title: "Memberships",
icon: :user_add,
url: Routes.membership_index_path(conn, :index, current_organization.id),
url: Routes.membership_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :board,
title: "Board",
icon: :users,
url: Routes.board_index_path(conn, :index, current_organization.id),
url: Routes.board_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
Expand All @@ -73,7 +76,7 @@ defmodule AtomicWeb.Config do
]
end

def default_pages(conn) do
def default_pages(conn, current_organization) do
[
%{
key: :home,
Expand All @@ -96,6 +99,33 @@ defmodule AtomicWeb.Config do
url: Routes.activity_index_path(conn, :index),
tabs: []
},
%{
key: :departments,
title: "Departments",
icon: :cube,
url: Routes.department_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :partners,
title: "Partners",
icon: :user_group,
url: Routes.partner_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :memberships,
title: "Memberships",
icon: :user_add,
url: Routes.membership_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :board,
title: "Board",
icon: :users,
url: Routes.board_index_path(conn, :index, current_organization),
},
Comment on lines +102 to +128
Copy link
Member

Choose a reason for hiding this comment

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

These are admin pages. Why are you adding them here?

%{
key: :announcements,
title: "Announcements",
Expand All @@ -113,14 +143,81 @@ defmodule AtomicWeb.Config do
]
end

def user_pages(conn, current_organization, current_user) do
[
%{
key: :home,
title: "Home",
icon: :home,
url: Routes.home_index_path(conn, :index),
tabs: []
},
%{
key: :calendar,
title: "Calendar",
icon: :calendar,
url: Routes.calendar_show_path(conn, :show),
tabs: []
},
%{
key: :activities,
title: "Activities",
icon: :academic_cap,
url: Routes.activity_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :departments,
title: "Departments",
icon: :cube,
url: Routes.department_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :partners,
title: "Partners",
icon: :user_group,
url: Routes.partner_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :board,
title: "Board",
icon: :users,
url: Routes.board_index_path(conn, :index, current_organization.slug),
tabs: []
},
%{
key: :organizations,
title: "Organizations",
icon: :office_building,
url: Routes.organization_index_path(conn, :index),
tabs: []
},
%{
key: :user,
title: "Profile",
icon: :user,
url: Routes.user_show_path(conn, :show, current_user.handle),
tabs: []
},
%{
key: :logout,
title: "Logout",
icon: :logout,
url: Routes.user_session_path(conn, :delete),
tabs: []
}
]
end
# Returns true if the user has permissions to access the organization admin pages
defp has_permissions?(_current_user, nil), do: false

defp has_permissions?(current_user, current_organization) do
Accounts.has_master_permissions?(current_user.id) ||
Accounts.has_permissions_inside_organization?(
current_user.id,
current_organization.id
current_organization.slug
)
end
end
4 changes: 2 additions & 2 deletions lib/atomic_web/live/activity_live/index.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<%= if not @empty? and @has_permissions? do %>
<div class="hidden lg:border-orange-500 lg:block">
<%= live_patch("+ New Activity",
to: Routes.activity_new_path(@socket, :new, @current_organization),
to: Routes.activity_new_path(@socket, :new, @current_organization.slug),
Copy link
Member

Choose a reason for hiding this comment

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

Additionally, interpolated ~p values are encoded via the Phoenix.Param protocol. For example, a %Post{} struct in your application may derive the Phoenix.Param protocol to generate slug-based paths rather than ID based ones. This allows you to use ~p"/posts/#{post}" rather than ~p"/posts/#{post.slug}" throughout your application. See the Phoenix.Param documentation for more details.

What about implementing this? Then we would only need to write @current_organization instead of @current_organization.slug 💡

Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to implement this 🙏🏼

class: "border-2 rounded-md bg-white text-lg border-orange-500 py-2 px-3.5 text-sm font-medium text-orange-500 shadow-sm hover:bg-orange-500 hover:text-white"
) %>
</div>
Expand Down Expand Up @@ -47,7 +47,7 @@
<!-- When there aren't any activities -->
<%= if @empty? and @has_permissions? do %>
<div class="mt-32">
<.empty_state id="empty_state" url={Routes.activity_new_path(@socket, :new, @current_organization)} plural_placeholder="activities" placeholder="activity" />
<.empty_state id="empty_state" url={Routes.activity_new_path(@socket, :new, @current_organization.slug)} plural_placeholder="activities" placeholder="activity" />
</div>
<% else %>
<!-- Activities listing -->
Expand Down
11 changes: 7 additions & 4 deletions lib/atomic_web/live/activity_live/show.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,21 @@ defmodule AtomicWeb.ActivityLive.Show do
alias Atomic.Activities.ActivityEnrollment

@impl true
def mount(%{"id" => id}, _session, socket) do
def mount(%{"slug" => slug, "id" => id}, _session, socket) do
if connected?(socket) do
Activities.subscribe("new_enrollment")
Activities.subscribe("deleted_enrollment")
end

{:ok, socket |> assign(:id, id)}
{:ok,
socket
|> assign(:id, id)
|> assign(:slug, slug)}
Comment on lines +17 to +18
Copy link
Member

Choose a reason for hiding this comment

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

Why are these assigns needed?

end

@impl true
def handle_params(%{"id" => id}, _, socket) do
activity = Activities.get_activity!(id, [:speakers, :organization])
def handle_params(%{"slug" => slug, "id" => id}, _, socket) do
activity = Activities.get_activity!(id, [:speakers, :departments])

entries = [
%{
Expand Down
9 changes: 8 additions & 1 deletion lib/atomic_web/live/announcement_live/index.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ defmodule AtomicWeb.AnnouncementLive.Index do
alias Atomic.Organizations

@impl true
def mount(_params, _session, socket) do
def mount(%{"slug" => slug}, _session, socket) do
organization = Organizations.get_organization_by_slug(slug)

socket =
socket
|> assign(:organization, organization)
|> assign(:announcements, Organizations.list_announcements_by_organization_id(organization.id))

{:ok, socket}
end

Expand Down
Loading