Skip to content

fix : restrict public templates list and view to importable types - #333

Open
mohammadsherif0 wants to merge 2 commits into
devfrom
feat-307-public-templates-list-view
Open

fix : restrict public templates list and view to importable types#333
mohammadsherif0 wants to merge 2 commits into
devfrom
feat-307-public-templates-list-view

Conversation

@mohammadsherif0

Copy link
Copy Markdown
Collaborator

Description

Fixes #307: non-admins could see and open public email templates in Public Templates even though they cannot import them. Listing and content view now follow the same importable-type rules as copy (types 4/5 for non-admins).

Improvements

Public Templates modal shows only importable public templates for non-admins (types 4 and 5); admins still see all public templates.
templateGetContent blocks non-owners from viewing public email templates unless they are admin.

@bingobongomann bingobongomann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two small improvements. One not even specific to this PR but generally to the template types arrays.

I could not reproduce the issue on the current dev branch

const isOwner = template.userId === this.userId;
const isPublicFromOthers = template.public === true && !isOwner;
const isAdmin = await this.isAdmin();
const isEmailType = [1, 2, 3, 6, 7].includes(template.type);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is the 9th or 10th time that we have the [1,...,7], [1,2,3,6,7] and [4,5] arrays in the template code. I think we should move this into some static lists like emailTemplateTypes = [1,2,3,6,7], otherTemplateTypes = [4,5], allTemplateTypes = emailTemplateTypes + otherTemplateTypes
And then use those statics lists. otherwise we have to adjust many different locations when adding new types in the future

*
* Fetches the template and returns its content as Quill Delta format for the given language.
* - For owners: returns stable content from template_content composed with draft edits (like documents)
* - For non-owners: returns only stable content (no drafts)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The docstring should reflect the rejection behavior for non-owners non-admins on email templates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] public templates modal should only list importable templates

2 participants