Add Message Templates management to Grand.Web.Store#694
Conversation
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/1b07a443-a39f-4582-9230-d1c8b6154d69 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/1b07a443-a39f-4582-9230-d1c8b6154d69 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
@copilot If message template exists for current store do not allow to add it again |
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/1c5b0e71-8c09-4c7a-bbfd-8e08e1a565ec Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Done in commit c81b017. The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
|
@copilot add missing resource string |
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/57979dac-6df7-46fc-90db-fa65d54059b1 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Added in commit e720b1a. The resource string Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot If we copy message template and exist for current store do not allow to add it again |
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/5f08e2f5-9eb0-4ed2-ac62-5e5dc47f2a11 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Done in commit f4681db. The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot now is new bug, if I try to copy template it always get a message "admin.content.messagetemplates.fields.name.alreadyexists" event if not exists! |
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/f3b9557e-207e-4413-9966-997bfeb76665 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Fixed in commit 54f4377. The root cause: The check now only blocks the copy if the found template has Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot missing resources: admin.content.messagetemplates.tab.store, admin.content.messagetemplates.tab.global, |
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/a78f4ba7-57ec-4186-8d1f-befdff60d57c Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Added in commit 74c5fa1. Both resource strings have been added to
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot copy is allowed only from templates where LimitedStore = false |
…Template action Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/d4201352-8e49-4a12-a9d0-e07d480710ea Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Done in commit fb220b8. The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot missing resource string: admin.content.messagetemplates.copied, and can still copy the same Name! |
…es.copied resource string Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/2421abf4-924b-47d0-b2c5-d0d54936ed31 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Fixed in commit 5195dd8.
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Adds a Store-area Message Templates management module so per-store staff can list, create, edit, delete, and copy message templates while only seeing/affecting templates scoped to their own store. Global templates are exposed read-only with a "Copy to my store" action.
Changes:
- New
MessageTemplateControllerenforcing store ownership for edit/delete/copy and exposing two grids (store-scoped, global). - New Razor views (
List,Create,Edit, partialCreateOrUpdate) wiring kendo grids, the localized editor, tokens, attachment, and delay UI. - (PR description) Adds
Admin.Content.MessageTemplates.*resource strings toDefaultLanguage.xml(not in the diff shown).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Web/Grand.Web.Store/Controllers/MessageTemplateController.cs | New controller for store-scoped message template CRUD plus copy-from-global. |
| src/Web/Grand.Web.Store/Areas/Store/Views/MessageTemplate/List.cshtml | Two-tab kendo grid view: store templates (CRUD) and global templates (read-only with per-row copy form). |
| src/Web/Grand.Web.Store/Areas/Store/Views/MessageTemplate/Create.cshtml | Wrapper view for creating a new store-specific template. |
| src/Web/Grand.Web.Store/Areas/Store/Views/MessageTemplate/Edit.cshtml | Wrapper view for editing with Save / Save&Continue / Copy / Delete buttons. |
| src/Web/Grand.Web.Store/Areas/Store/Views/MessageTemplate/Partials/CreateOrUpdate.cshtml | Shared form partial with localized editor, token picker, send delay, attachment fields. |
Comments suppressed due to low confidence (6)
src/Web/Grand.Web.Store/Controllers/MessageTemplateController.cs:268
- CopyTemplate's duplicate-name check is broken.
GetMessageTemplateByName(name, storeId)filters results throughIAclService.Authorize, which returns true for any template whereLimitedToStores == false(seeAclService.Authorize). The global template being copied therefore matches its own name lookup, soexistingis never null and every copy attempt fails with "Admin.Content.MessageTemplates.Fields.Name.AlreadyExists". To detect only store-specific duplicates for the current store (as the PR description states), iterateGetAllMessageTemplates("")and look for a template whereLimitedToStores == true && Stores.Contains(CurrentStoreId) && Name == messageTemplate.Name(or otherwise exclude global matches).
// Prevent duplicate: check if a store-specific template with the same name already exists for the current store
var existing = await messageTemplateService.GetMessageTemplateByName(messageTemplate.Name, CurrentStoreId);
if (existing != null)
{
Error(translationService.GetResource("Admin.Content.MessageTemplates.Fields.Name.AlreadyExists"));
return RedirectToAction("List");
}
src/Web/Grand.Web.Store/Controllers/MessageTemplateController.cs:252
- CopyTemplate is gated by
PermissionActionName.Edit, but the action actually creates a new entity (it inserts a copy viaCopyMessageTemplateand only then updates the store assignment). For consistency with other create flows in this controller and the rest of the Store area, consider gating it withPermissionActionName.Createinstead, so that a user with view/edit but not create permission cannot spawn new templates.
[PermissionAuthorizeAction(PermissionActionName.Edit)]
[HttpPost]
public async Task<IActionResult> CopyTemplate(MessageTemplateModel model)
src/Web/Grand.Web.Store/Controllers/MessageTemplateController.cs:279
CopyMessageTemplatecopiesStoresandLimitedToStoresfrom the source. Because the source is always a global template (LimitedToStores == false), the inserted copy is briefly visible globally to all stores betweenInsertMessageTemplateand the subsequentUpdateMessageTemplatethat re-scopes it to the current store. SettingLimitedToStores = trueandStores = [CurrentStoreId]on the in-memory entity before persisting (or doing the assignment inside a single insert) would avoid the temporary cross-store exposure and remove a window where another store could load the new template via cache.
try
{
var newMessageTemplate = await messageTemplateService.CopyMessageTemplate(messageTemplate);
// Assign copy to the current store
newMessageTemplate.LimitedToStores = true;
newMessageTemplate.Stores = [CurrentStoreId];
await messageTemplateService.UpdateMessageTemplate(newMessageTemplate);
Success(translationService.GetResource("Admin.Content.MessageTemplates.Copied"));
return RedirectToAction("Edit", new { id = newMessageTemplate.Id });
src/Web/Grand.Web.Store/Controllers/MessageTemplateController.cs:196
messageTemplate.Stores = [CurrentStoreId];unconditionally collapses the assigned-store collection to a single entry. If a store-specific template was ever assigned to multiple stores (e.g. a future admin enhancement), an edit by a staff user would silently strip all other stores from it. Consider preserving any other entries and only ensuringCurrentStoreIdis included, or at minimum documenting the single-store assumption.
// Keep it assigned to the current store
messageTemplate.LimitedToStores = true;
messageTemplate.Stores = [CurrentStoreId];
src/Web/Grand.Web.Store/Controllers/MessageTemplateController.cs:256
- When the staff user uses the "Copy" button on the Edit page, this submits the standard Edit
<form>withformaction=CopyTemplate. The form does not include the source template'sNamefield as a hidden input when editing (line 65 only emits the hiddenNamebecause the template already has an Id), but theBodyfield plus a number of other model properties WILL be submitted unfiltered toCopyTemplate(MessageTemplateModel model). The action only readsmodel.Idso this is harmless today, but it means the Copy button effectively bypasses normal model validation; if a future change starts using fields offmodelhere, that data would come from a partially-edited form rather than the original entity. Worth either narrowing the action's parameter to juststring idor documenting the intent.
public async Task<IActionResult> CopyTemplate(MessageTemplateModel model)
{
var messageTemplate = await messageTemplateService.GetMessageTemplateById(model.Id);
if (messageTemplate == null)
return RedirectToAction("List");
src/Web/Grand.Web.Store/Areas/Store/Views/MessageTemplate/Partials/CreateOrUpdate.cshtml:213
- The kendo column template builds a regex from the user's search input using
new RegExp(searchVal, "i")without escaping. If a token name (or, more likely, a typed search character) contains a regex metacharacter such as(,[,*, etc., this will throw aSyntaxErrorand break the search behavior on every subsequent keystroke. Escape the input before constructing the regex (e.g.searchVal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) or useindexOf/includesinstead.
function SearchTokens(e) {
var searchVal = $(e.target).val();
var TokensPnl = $(e.target).closest(".form-group").find(".AllowedTokens");
$(TokensPnl).find(".btn").show()
.each(function (i, c) {
var Token = $(c).text();
if (Token.search(new RegExp(searchVal, "i")) == -1) {
$(c).hide();
}
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var existing = await messageTemplateService.GetMessageTemplateByName(model.Name, CurrentStoreId); | ||
| if (existing != null) | ||
| { | ||
| ModelState.AddModelError("Name", translationService.GetResource("Admin.Content.MessageTemplates.Fields.Name.AlreadyExists")); | ||
| model.HasAttachedDownload = !string.IsNullOrEmpty(model.AttachedDownloadId); | ||
| model.AllowedTokens = messageTokenProvider.GetListOfAllowedTokens(); | ||
| foreach (var ea in await emailAccountService.GetAllEmailAccounts(CurrentStoreId)) | ||
| model.AvailableEmailAccounts.Add(ea.ToModel()); | ||
| return View(model); | ||
| } |
There was a problem hiding this comment.
Fixed in commit 2668755. The Create duplicate check now uses GetAllMessageTemplates("") and filters explicitly for LimitedToStores = true && Stores.Contains(CurrentStoreId) — the same pattern already in CopyTemplate. This means a store user can create a template named "OrderPlaced.CustomerNotification" even though a global template with that name exists; only an existing store-specific copy of that name blocks creation.
| public async Task<IActionResult> ListStore(DataSourceRequest command) | ||
| { | ||
| var allTemplates = await messageTemplateService.GetAllMessageTemplates(CurrentStoreId); | ||
| var storeTemplates = allTemplates | ||
| .Where(t => t.LimitedToStores && t.Stores.Contains(CurrentStoreId)) | ||
| .ToList(); | ||
|
|
||
| var items = storeTemplates.Select(x => x.ToModel()).ToList(); | ||
|
|
||
| var gridModel = new DataSourceResult { | ||
| Data = items, | ||
| Total = items.Count | ||
| }; | ||
|
|
||
| return Json(gridModel); | ||
| } |
| public async Task<IActionResult> ListGlobal(DataSourceRequest command) | ||
| { | ||
| var allTemplates = await messageTemplateService.GetAllMessageTemplates(""); | ||
| var globalTemplates = allTemplates | ||
| .Where(t => !t.LimitedToStores) | ||
| .ToList(); | ||
|
|
||
| var items = globalTemplates.Select(x => x.ToModel()).ToList(); | ||
|
|
||
| var gridModel = new DataSourceResult { | ||
| Data = items, | ||
| Total = items.Count | ||
| }; | ||
|
|
||
| return Json(gridModel); | ||
| } |
There was a problem hiding this comment.
Done in commit 3cd28d8. Both ListGlobal and ListStore now apply Skip((command.Page - 1) * command.PageSize).Take(command.PageSize) before projecting to models and return the full count in Total. The List.cshtml view now injects AdminAreaSettings and sets pageSize: @(adminAreaSettings.DefaultGridPageSize) and pageSizes: [@(adminAreaSettings.GridPageSizes)] on both grids, matching the pattern used by other Store-area list views.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/40ee3d5e-fc73-4a47-a5ca-f3d900d30869 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…ecific templates with same name as global templates Agent-Logs-Url: https://github.com/grandnode/grandnode2/sessions/e7f3eee3-ad46-44a1-834c-44b0a06d6341 Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
MessageTemplateControllerinGrand.Web.Store/Controllers/command.Page/command.PageSizefor server-side paginationGrand.Web.Store/Areas/Store/Views/MessageTemplate/views:DefaultLanguage.xml