Conversation
Added an `<ItemGroup>` to the project file to include an embedded resource for `Messages.resx` and a compile entry for `Messages.Designer.cs`. This enables processing of the resource file by the `PublicResXFileCodeGenerator` and configures properties for design-time support and auto-generation of the designer file.
Refactor `CreateCategoryVM` and `UpdateCategoryVM` to use resource-based error messages for validation attributes. Added `using Refhub.Resources` and replaced hardcoded strings with `ErrorMessageResourceType` and `ErrorMessageResourceName` for improved localization support and consistency in error messaging.
Updated validation attributes to use resource-based error messages for better localization. Changed Email, Password, and ConfirmPassword properties to use the 'required' keyword to enforce mandatory fields.
Updated `KeywordService` to inject `IMessageService` for dynamic error messages. Refactored user not found error handling in `UserService` to utilize the same service, improving localization and message management.
Refactor the `Word` property validation in `CreateKeywordVM` and `EditKeywordVM` to use resource localization. Replace hardcoded Persian error messages with references to the `Messages` class for improved internationalization and maintainability.
This commit introduces new localized string resources in the `Messages.Designer.cs` file, along with corresponding entries in the `Messages.fa.resx` and `Messages.resx` files. The added properties provide error messages and prompts for user input, including password confirmation, email validation, and category descriptions, improving user feedback in both Persian and English.
labels-and-badges
bot
added
NO JIRA
WalkthroughThis update introduces comprehensive multi-language support and centralized message localization across the application. It adds resource files for English and Persian, a message service abstraction, and modifies validation and error messages to use localized resources. Database schema changes enable soft deletion for books and enforce unique slugs only for non-deleted entries. Controller and service logic are updated for localization and improved message handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant LanguageController
participant Cookie
participant HomeController
User->>Browser: Clicks /fa or /en
Browser->>LanguageController: HTTP GET /fa or /en
LanguageController->>Cookie: Set culture cookie (fa-IR or en-US)
LanguageController->>HomeController: Redirect to Index
HomeController-->>User: Rendered home page in selected language
sequenceDiagram
participant Service
participant IMessageService
participant ResourceFile
Service->>IMessageService: Get("Error_Key")
IMessageService->>ResourceFile: Lookup "Error_Key"
ResourceFile-->>IMessageService: Return localized string
IMessageService-->>Service: Return localized string
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @abolfazlshs80, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request implements multi-language capabilities across the application, primarily focusing on localizing user-facing messages, especially validation errors, by integrating ASP.NET Core's localization framework and resource files. It also includes a database schema update for the Book entity's slug.
Highlights
- Multi-Language Implementation: Introduced multi-language support using ASP.NET Core's built-in localization features, including resource files (.resx) for storing localized strings.
- Localized Validation Messages: Updated various ViewModel classes (AuthorVM, CreateBookVM, UpdateBookVM, CreateCategoryVM, UpdateCategoryVM, CreateKeywordVM, EditKeywordVM, LoginVM, RegisterVM) to use localized error messages defined in resource files for data annotations.
- Language Switching Controller: Added a new
LanguageControllerwith endpoints (/fa,/en) to allow users to switch languages by setting a culture cookie, and a/TestMessageendpoint to demonstrate localized message retrieval. - Localization Service: Created an
IMessageServiceinterface and its implementationMessageServiceto provide a centralized way to retrieve localized strings within application services. - Database Migration: Included a database migration (
fix-mig) that modifies theBookstable, changing theSlugcolumn type tonvarchar(450)and adding a unique index to enforce uniqueness and improve lookup performance.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces multi-language support to the application, a valuable enhancement. The core changes involve setting up localization services, adding resource files for English and Persian, creating a MessageService for accessing localized strings, and updating ViewModels and other services to utilize these localized messages. A new LanguageController allows users to switch between supported languages.
The review has identified a critical compilation error that needs to be addressed. Additionally, there are high and medium severity issues concerning incorrect error messaging, maintainability, and consistency. These should be reviewed and resolved to ensure the quality and correctness of the implementation.
The PR also includes database migration files (20250611081248_fix-mig) that modify the Books.Slug column. While these schema changes appear beneficial, they are unrelated to the multi-language feature. For better clarity and easier review, it's generally recommended to separate unrelated changes into distinct pull requests.
As no specific style guide was provided, the review feedback adheres to general C# and .NET coding conventions, such as those outlined by Microsoft, focusing on clarity, maintainability, and correctness.
| { | ||
| option.ResourcesPath = "Resources"; | ||
| }).AddDataAnnotationsLocalization(); | ||
| ; |
There was a problem hiding this comment.
There is an extraneous semicolon ;; at the end of the AddDataAnnotationsLocalization() call. This will cause a compilation error.
}).AddDataAnnotationsLocalization();| if (exists) | ||
| { | ||
| throw new Exception("این کلیدواژه قبلاً ثبت شده است."); | ||
| throw new Exception(_messageService.Get("Keyword_Notfound")); |
There was a problem hiding this comment.
The resource key "Keyword_Notfound" is used here when an attempt is made to add a keyword that already exists. However, the messages associated with Keyword_Notfound (e.g., "Keyword notfound" in English, "کلید واژه یافت نشد ." in Persian) imply that a keyword was not found, which is the opposite of the actual situation (keyword already exists).
This will result in a misleading error message for the user. It's recommended to create and use a new resource key, such as Keyword_AlreadyExists, with an appropriate message like "This keyword has already been registered."
| public IActionResult TestMessage() | ||
| { | ||
|
|
||
| return Content(messageService.Get("Title")); | ||
| } |
There was a problem hiding this comment.
The TestMessage action method appears to be for testing or debugging purposes. If this endpoint is not intended for production use, consider removing it before merging to avoid potential misuse or exposure of internal details. If it serves a specific diagnostic purpose in production, ensure it is adequately secured and documented.
| namespace Refhub.Migrations | ||
| { | ||
| /// <inheritdoc /> | ||
| public partial class fixmig : Migration |
There was a problem hiding this comment.
The migration class name fixmig is quite generic. Migration names should be descriptive of the changes they implement (e.g., AddUniqueIndexToBookSlugAndModifyType). This practice significantly improves maintainability and makes it easier for developers to understand the evolution of the database schema by just looking at migration names.
public partial class AddUniqueIndexToBookSlugAndModifyType : Migration
Refhub/Resources/Messages.resx
Outdated
| <value>The Email is not Valid !</value> | ||
| </data> | ||
| <data name="Keyword_Required" xml:space="preserve"> | ||
| <value>chooese a keyword !</value> |
There was a problem hiding this comment.
The value for the resource key Keyword_Required ("�chooese a keyword !") starts with an XML character entity , which represents a backspace character. This is likely unintentional and could lead to unexpected rendering issues or behavior. Please remove this character.
Additionally, "chooese" appears to be a typo for "choose".
<data name="Keyword_Required" xml:space="preserve">
<value>choose a keyword !</value>
</data>
| return error.Code is "DuplicateUserName" or "DuplicateEmail" | ||
| ? (ErrorOr<RegisterVM>)Error.Validation(ErrorMessageAuthenticationStatic.Error_Invalid_Email) | ||
| : (ErrorOr<RegisterVM>)Error.Validation(ErrorMessageAuthenticationStatic.Error_Register); | ||
| ? (ErrorOr<RegisterVM>)Error.Validation(_messageService.Get("The email entered is already registered.")) |
There was a problem hiding this comment.
Using a full string literal "The email entered is already registered." as a key for _messageService.Get() is not a recommended practice. Resource keys should be stable identifiers (e.g., Account_EmailAlreadyRegistered). This improves maintainability, reduces the risk of typos, and makes it easier to manage translations.
Your Messages.resx file already defines Account_EmailAleady for this purpose. Please use that key.
? (ErrorOr<RegisterVM>)Error.Validation(_messageService.Get("Account_EmailAleady"))| using Refhub.Service.Interface; | ||
|
|
||
|
|
||
| namespace Refhub_Ir.Tools.ExtentionMethod; |
There was a problem hiding this comment.
The namespace Refhub_Ir.Tools.ExtentionMethod includes _Ir, which seems inconsistent with the main project namespace Refhub. For better project structure and consistency, consider renaming this namespace to something like Refhub.Tools.ExtensionMethods or Refhub.Extensions.
namespace Refhub.Tools.ExtensionMethods;![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
Refhub/Models/Category/UpdateCategoryVM.cs (1)
9-17: Same key-mapping concerns as inCreateCategoryVMThe warning on confusing or shared resource keys applies here as well.
🧹 Nitpick comments (15)
Refhub/Tools/ExtentionMethod/AddServiceExtentionMethod.cs (1)
16-16: ConsiderSingletonscope for a statelessMessageService.
MessageServiceis typically a thin wrapper overResourceManager/IStringLocalizerand contains no per-request state. Registering it asAddScopedincurs per-request instantiation overhead with no benefit.-collection.AddScoped<IMessageService, MessageService>(); +collection.AddSingleton<IMessageService, MessageService>();If the implementation does keep request–specific data (e.g. culture coming from
IHttpContextAccessor), keep the scoped lifetime; otherwise switch to singleton.Refhub/Models/Keywords/CreateKeywordVM.cs (1)
8-10: Satisfy nullable-reference-type analysis forWord.
stringis non-nullable, yet the object can be instantiated without settingWord, producing CS8618 warnings. Initialise with an empty string to silence the warning and keep the[Required]runtime check.-public string Word { get; set; } +public string Word { get; set; } = string.Empty;Refhub/Models/Authors/AuthorVM.cs (1)
8-15: Initialise non-nullable properties to avoid CS8618 warnings.Same rationale as for
CreateKeywordVM.-public string FullName { get; set; } +public string FullName { get; set; } = string.Empty; -public string Slug { get; set; } +public string Slug { get; set; } = string.Empty;Refhub/Program.cs (1)
4-4: Avoid introducing a second “Tools” namespace – consolidate or rename for clarity
Refhub_Ir.Tools.ExtentionMethodis very close to the existingRefhub.Tools.ExtentionMethod. Two namespaces that differ only by an_Irsuffix is a breeding-ground for “which one do I import?” bugs.
Unless there is a very strong reason (separate assembly, legal, etc.), consider merging the extension methods under a single namespace or using a clearer qualifier (e.g.,.Localization).Refhub/Models/Category/CreateCategoryVM.cs (2)
8-13: Resource key looks unrelated to the property – probable copy/paste slip
Nameis annotated withMessages.Book_Title_CategoryRequired.
That key name reads like “Book Title Category Required” and does not obviously map to a Category’s Name.
Consider introducing a clearer key, e.g.Category_Name_Required, to avoid confusion for translators and future maintainers.-[Required(ErrorMessageResourceType = typeof(Messages), - ErrorMessageResourceName = nameof(Messages.Book_Title_CategoryRequired))] +[Required(ErrorMessageResourceType = typeof(Messages), + ErrorMessageResourceName = nameof(Messages.Category_Name_Required))]
12-13: Double-duty key for “Name / Slug” weakens future flexibilityUsing a single key
Name_Slug_Requiredties two semantically different fields together. If one day you need a different wording for “Slug is required” (e.g., mention only lowercase letters), you will be forced to create a new key and update code anyway. Define one key per field up-front.Refhub/Service/Implement/AuthorService.cs (1)
88-89: Inconsistent casing in resource keys (Notfound,NotFound)Two variants are used:
Error_AuthorNotfoundandError_AuthorNotFound.
Standardise the key to a single spelling to avoid silent fall-backs.-throw new Exception(_messageService.Get("Error_AuthorNotfound")); +throw new Exception(_messageService.Get("Error_AuthorNotFound"));(Apply the same change in all occurrences.)
Also applies to: 108-109, 114-115, 128-129
Refhub/Service/Implement/MessageService.cs (1)
12-17: Simplify and make assembly discovery saferIf
MessageServiceever moves to another assembly,Assembly.GetExecutingAssembly()will no longer match the assembly that hostsMessages.resx, causing fallback[key]everywhere. Prefer tying the localizer directly to the resource type:-var assemblyName = Assembly.GetExecutingAssembly().GetName().Name ?? "Refhub"; -_localizer = factory.Create(nameof(Messages), assemblyName); +_localizer = factory.Create(typeof(Messages));
IStringLocalizerFactory.Create(Type resourceSource)is available in ASP.NET Core 8 and avoids hard-coding the assembly name.Refhub/Controllers/LanguageController.cs (2)
8-9: Inconsistent naming & unused usingController class and action names should follow .NET PascalCase conventions.
Renamepersian()→Persian,english()→Englishto match routing defaults and avoid surprises when generating links.Also,
using Refhub.Service.Implement;is unused and can be removed.
15-18: Set Secure / SameSite on culture cookiesFor cookies that live a whole year, it is best practice to set security attributes explicitly:
-new CookieOptions() { Expires = DateTimeOffset.UtcNow.AddYears(1) }); +new CookieOptions +{ + Expires = DateTimeOffset.UtcNow.AddYears(1), + IsEssential = true, + SameSite = SameSiteMode.Lax, + Secure = true +});Avoids lax-same-site default surprises and mixed-content leaks when the site is served over HTTPS.
Also applies to: 23-26
Refhub/Models/Users/LoginVM.cs (1)
8-14: Consider adding display names for e-mail fieldYou swapped hard-coded messages for resources (👍) but the
[Display]attribute was removed.
Without it, scaffolding / validation summaries show the raw property name
Add[Display(Name = nameof(Messages.Email_Display), ResourceType = typeof(Messages))].Refhub/Tools/ExtentionMethod/MultiLanguageExtentionMethod.cs (3)
10-10: Typo: “Extention” → “Extension”The class and namespace names contain a spelling mistake.
Renaming early avoids ripple effects and keeps API surface professional.
37-41: Remove stray semicolonLine 41 is an empty statement – harmless but noisy.
- }).AddDataAnnotationsLocalization(); - ; + }).AddDataAnnotationsLocalization();
24-28: AddAcceptLanguageHeaderRequestCultureProviderfor better UXRelying only on query-string / cookie ignores browser preferences.
Appending the default header provider keeps explicit providers’ priority while giving sensible fallback.- RequestCultureProviders = new List<IRequestCultureProvider>() + RequestCultureProviders = new List<IRequestCultureProvider>(){ new QueryStringRequestCultureProvider(), new CookieRequestCultureProvider(), new AcceptLanguageHeaderRequestCultureProvider() }Refhub/Resources/Messages.Designer.cs (1)
64-70: Spelling / consistency issues in resource keysSeveral identifiers contain typos that will leak into code and localisation:
Account_EmailAleady→Account_EmailAlreadyError_NotDefinde→Error_NotDefinedKeyword_Notfound→Keyword_NotFoundKeyword_Requiredtext includes stray control character (�chooese).Correcting early avoids breaking consumers and resource duplication.
Also applies to: 228-231, 264-267
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
Refhub/Controllers/LanguageController.cs(1 hunks)Refhub/Migrations/20250611081248_fix-mig.Designer.cs(1 hunks)Refhub/Migrations/20250611081248_fix-mig.cs(1 hunks)Refhub/Migrations/AppDbContextModelSnapshot.cs(3 hunks)Refhub/Models/Authors/AuthorVM.cs(1 hunks)Refhub/Models/Books/CreateBookVM.cs(1 hunks)Refhub/Models/Books/UpdateBookVM.cs(2 hunks)Refhub/Models/Category/CreateCategoryVM.cs(1 hunks)Refhub/Models/Category/UpdateCategoryVM.cs(1 hunks)Refhub/Models/Keywords/CreateKeywordVM.cs(1 hunks)Refhub/Models/Keywords/EditKeywordVM.cs(1 hunks)Refhub/Models/Users/LoginVM.cs(1 hunks)Refhub/Models/Users/RegisterVM.cs(1 hunks)Refhub/Program.cs(3 hunks)Refhub/Refhub.csproj(1 hunks)Refhub/Resources/Messages.Designer.cs(1 hunks)Refhub/Resources/Messages.fa.resx(1 hunks)Refhub/Resources/Messages.resx(1 hunks)Refhub/Service/Implement/AuthorService.cs(5 hunks)Refhub/Service/Implement/KeywordService.cs(1 hunks)Refhub/Service/Implement/MessageService.cs(1 hunks)Refhub/Service/Implement/UserService.cs(4 hunks)Refhub/Service/Interface/IMessageService.cs(1 hunks)Refhub/Tools/ExtentionMethod/AddServiceExtentionMethod.cs(1 hunks)Refhub/Tools/ExtentionMethod/MultiLanguageExtentionMethod.cs(1 hunks)Refhub/Tools/Static/AuthorMessage.cs(0 hunks)Refhub/Tools/Static/ErrorMessageAuthenticationStatic.cs(0 hunks)
💤 Files with no reviewable changes (2)
- Refhub/Tools/Static/ErrorMessageAuthenticationStatic.cs
- Refhub/Tools/Static/AuthorMessage.cs
🧰 Additional context used
🧬 Code Graph Analysis (6)
Refhub/Models/Category/CreateCategoryVM.cs (1)
Refhub/Data/Models/Category.cs (1)
Category(3-12)
Refhub/Models/Category/UpdateCategoryVM.cs (1)
Refhub/Data/Models/Category.cs (1)
Category(3-12)
Refhub/Service/Implement/KeywordService.cs (1)
Refhub/Models/Keywords/CreateKeywordVM.cs (1)
CreateKeywordVM(6-11)
Refhub/Service/Implement/UserService.cs (3)
Refhub/Models/Users/UserListAdminVM.cs (1)
UserListAdminVM(3-10)Refhub/Models/Users/LoginVM.cs (1)
LoginVM(6-30)Refhub/Models/Users/RegisterVM.cs (1)
RegisterVM(6-34)
Refhub/Service/Implement/AuthorService.cs (2)
Refhub/Data/Models/Book.cs (1)
Book(3-26)Refhub/Data/Models/Author.cs (1)
Author(3-12)
Refhub/Migrations/20250611081248_fix-mig.Designer.cs (2)
Refhub/Migrations/AppDbContextModelSnapshot.cs (1)
DbContext(13-537)Refhub/Migrations/20250611081248_fix-mig.cs (1)
fixmig(8-43)
🪛 GitHub Actions: 🚀 Build Pipeline
Refhub/Models/Users/RegisterVM.cs
[error] 28-28: Compiler error CS1026: ')' expected.
[error] 32-32: Compiler error CS1026: ')' expected.
🔇 Additional comments (12)
Refhub/Refhub.csproj (1)
26-36:Details
❓ Verification inconclusive
Ensure satellite resource (.fa) is included.
Only the neutral
Messages.resxis embedded. To generate the Persian satellite assembly add an item forMessages.fa.resx; otherwise localisation will silently fall back to the neutral culture.<ItemGroup> + <EmbeddedResource Include="Resources\Messages.fa.resx" /> </ItemGroup>
#!/bin/bash echo "Checking for Persian resource file in repository:" find . -type f -iname "Messages.fa.resx" echo "" echo "Inspecting Refhub.csproj for any .fa.resx entries:" grep -n "<EmbeddedResource.*fa\.resx" -n Refhub/Refhub.csproj || true
#!/bin/bash echo "Searching for Persian resource file in the repository:" fd --ignore-case 'messages.fa.resx' || echo "No Messages.fa.resx file found" echo "" echo "Checking Refhub.csproj for any .fa.resx entries:" grep -n 'EmbeddedResource.*fa\.resx' Refhub/Refhub.csproj || echo "No .fa.resx entries in Refhub.csproj"
#!/bin/bash echo "Locating Refhub.csproj in the repository:" fd --hidden --exclude node_modules Refhub.csproj || echo "Refhub.csproj not found" echo "" echo "Searching for any Persian resource files (*.fa.resx):" fd --hidden --exclude node_modules -I '*.fa.resx' || echo "No *.fa.resx files found"
Verify Persian resource inclusion
It looks like there’s no
Messages.fa.resxin the repo (and none in Refhub/Refhub.csproj). If you intend to ship a Persian satellite assembly:• Ensure
Resources/Messages.fa.resxexists.
• Add it to your project file under an<ItemGroup>, for example:<ItemGroup> <EmbeddedResource Include="Resources\Messages.fa.resx" /> </ItemGroup>Refhub/Refhub.csproj:26-36 – confirm or add the above entry.
Refhub/Service/Interface/IMessageService.cs (1)
3-7: Consider unifying / renaming the overloads.
Get(string key)is effectively covered byGet(string key, params object[] parameters)– a call with no extra parameters resolves to theparamsoverload.
Keeping both overloads adds a redundant API surface and potential ambiguity for consumers using IDE auto-complete.-string Get(string key); -string Get(string key, params object[] parameters); +string Get(string key, params object[] parameters);If you want the simple/no-params call for readability, at minimum add XML-docs clarifying the difference and expected usage scenarios.
[ suggest_optional_refactor ]Refhub/Service/Implement/KeywordService.cs (3)
13-19: Guard-clause the injected service.
_messageServiceis assigned without null-checking. Although DI normally guarantees non-null, a defensiveArgumentNullException.ThrowIfNull(messageService);protects against mis-configuration and aids unit testing.
[ suggest_nitpick ]
23-27: Incorrect message key & generic exception type.
- The logic enters this block when the keyword already exists, yet the key used is
"Keyword_Notfound".- Throwing
Exceptionloses semantic meaning and hampers global error handling.-throw new Exception(_messageService.Get("Keyword_Notfound")); +throw new InvalidOperationException(_messageService.Get("Keyword_Exists"));(Adjust the resource key to whatever naming you settle on.)
Using a specific exception (or a custom
ConflictException) improves clarity and enables consumers/middleware to map 409 responses.
[ raise_critical_issue ]
23-24: AvoidToLower()in EF predicate.
ToLower()prevents index usage and adds CPU overhead. Prefer case-insensitive comparison:var exists = await _context.Keywords .AnyAsync(k => k.Word.Equals(model.Word, StringComparison.OrdinalIgnoreCase), ct);That keeps the expression client-side-free and index-friendly (works on SQL Server 2019+ with proper collation).
[ suggest_essential_refactor ]Refhub/Migrations/20250611081248_fix-mig.cs (1)
11-26: Possible data-loss & migration failure risks.Changing
Slugfromnvarchar(max)tonvarchar(450)+ UNIQUE will:
- Truncate any existing slugs > 450 chars (SQL silently cuts them).
- Fail the migration if duplicates already exist.
Mitigate by:
• Pre-checking length and duplicates in a custom SQL step, or
• Truncating/normalising data before applying the UNIQUE constraint.Ask DBA / ops to verify production data before running.
[ request_verification ]Refhub/Models/Books/UpdateBookVM.cs (2)
11-13:[Required]on non-nullable value types is redundant.
PageCountis anint; the CLR already disallowsnull. Instead validate range:-[Required(ErrorMessageResourceType = typeof(Messages), - ErrorMessageResourceName = nameof(Messages.Book_PageCountRequired))] +[Range(1, int.MaxValue, + ErrorMessageResourceType = typeof(Messages), + ErrorMessageResourceName = nameof(Messages.Book_PageCountPositive))][ suggest_essential_refactor ]
35-37: Ensure collection is non-empty.
List<int>is nevernullwhen model-bound, so[Required]won’t catch an empty list. Add a length validator:[MinLength(1, ErrorMessageResourceType = typeof(Messages), ErrorMessageResourceName = nameof(Messages.Book_AnotherIdRequired))] public List<int> AnotherId { get; set; } = new();[ suggest_optional_refactor ]
Refhub/Models/Keywords/EditKeywordVM.cs (1)
10-12: Complement[Required]with length constraints.Add
[StringLength]or[MaxLength]to align with DB column and prevent overly long keywords:[Required(ErrorMessageResourceType = typeof(Messages), ErrorMessageResourceName = nameof(Messages.Keyword_Required))] [StringLength(100)] public string Word { get; set; }[ suggest_optional_refactor ]
Refhub/Program.cs (2)
16-16: Confirm thatAddMultiLanguage()wires upAddLocalization()/ view-localizationThe call chain assumes that the extension returns an
IMvcBuilderand registers:•
services.AddLocalization(options => …)
•mvcBuilder.AddViewLocalization()/AddDataAnnotationsLocalization()If any of those are missing, resource look-ups for validation attributes will fail at runtime with a generic “could not find resource” exception.
53-53: Middleware ordering is correct – good job
UseMultiLanguage()is inserted beforeUseRouting(), which is exactly whereUseRequestLocalization()is expected to run.Refhub/Migrations/20250611081248_fix-mig.Designer.cs (1)
1-540: Generated file – no manual changes requiredDesigner code is auto-generated by EF; review skipped.
| b.Property<string>("Slug") | ||
| .IsRequired() | ||
| .HasColumnType("nvarchar(max)"); | ||
| .HasColumnType("nvarchar(450)"); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add matching validation limits for Slug length in the application layer.
Slug is now limited to nvarchar(450) and is unique.
Please ensure:
- All DTO / ViewModel classes (
CreateBookVM,UpdateBookVM, etc.) enforceMaxLength(450)so client-side validation aligns with the DB constraint. - Existing data is migrated (duplicates / >450 chars will break the migration).
Also applies to: 283-285
🏁 Script executed:
#!/bin/bash
# Scan ViewModels for a MaxLength attribute on Slug
rg -A2 -I $'Slug.*MaxLength' | headLength of output: 36
I’ll locate where Slug is defined across C# files to check for missing validation attributes.
🏁 Script executed:
#!/bin/bash
# Search for Slug property definitions in C# files
rg -H -n "public.*Slug.*{ get; set; }" -t cs | head -n 20Length of output: 185
Let’s expand the search to list all C# files (outside of Migrations) that reference Slug, then check for any [MaxLength] or [StringLength] annotations nearby.
🏁 Script executed:
#!/bin/bash
# 1. List all C# files (excluding migrations) containing "Slug"
echo "→ Files containing Slug (excluding Migrations):"
rg -l "Slug" -t cs --ignore-dir Migrations
# 2. In those files, look for data-annotation attributes around Slug
echo -e "\n→ [MaxLength] usages near Slug:"
rg -n -H "\[MaxLength\(" -t cs --ignore-dir Migrations | grep -C2 "Slug" || echo " (none found)"
echo -e "\n→ [StringLength] usages near Slug:"
rg -n -H "\[StringLength\(" -t cs --ignore-dir Migrations | grep -C2 "Slug" || echo " (none found)"Length of output: 1867
Let’s search all C# files (outside the Migrations folder) that reference Slug and check for any [MaxLength] or [StringLength] annotations on it:
🏁 Script executed:
#!/bin/bash
# 1. List all C# files referencing 'Slug' (exclude migrations)
echo "Files containing 'Slug' (excluding migrations):"
files=$(rg -l 'Slug' -t cs | grep -v -i 'Migrations')
echo "$files"
# 2. For each file, look for data‐annotation attributes around the Slug property
echo -e "\nChecking for [MaxLength] and [StringLength] near 'Slug':"
for f in $files; do
echo "→ $f"
rg -n '\[MaxLength\([0-9]+' "$f" -C2 || echo " No [MaxLength] found"
rg -n '\[StringLength\([0-9]+' "$f" -C2 || echo " No [StringLength] found"
doneLength of output: 13605
Add length validation to all Slug properties in DTO/ViewModel classes
The database now enforces nvarchar(450) on Slug, but none of the application-level models include a matching [MaxLength(450)] (or [StringLength(450)]). Without these, client-side validation won’t catch too-long values and submissions may fail at save time.
Please update the following classes to decorate the Slug property:
- Refhub/Models/Books/CreateBookVM.cs
- Refhub/Models/Books/UpdateBookVM.cs
- Refhub/Models/Books/BookDetailsVM.cs
- Refhub/Models/Books/BooksList_VM.cs
- Refhub/Models/Books/BookVM.cs
- Refhub/Models/Books/BookItemVM.cs
- Refhub/Models/Category/CategoryVM.cs
- Refhub/Models/Category/CreateCategoryVM.cs
- Refhub/Models/Category/UpdateCategoryVM.cs
- Refhub/Models/Authors/AuthorVM.cs
Example change in each class:
public class CreateBookVM
{
+ [MaxLength(450)]
public string Slug { get; set; }
// …
}Also ensure any other models or DTOs that expose Slug follow the same pattern.
Finally, audit existing data before applying the migration—truncate or resolve any Slug values longer than 450 characters (and handle duplicates) so the schema update succeeds.
🤖 Prompt for AI Agents
In Refhub/Migrations/AppDbContextModelSnapshot.cs around lines 267 to 270 and
283 to 285, the Slug property is limited to nvarchar(450) in the database but
the application layer DTO/ViewModel classes lack matching MaxLength or
StringLength validation attributes. To fix this, add a [MaxLength(450)] or
[StringLength(450)] attribute to the Slug property in all listed DTO/ViewModel
classes such as CreateBookVM.cs, UpdateBookVM.cs, BookDetailsVM.cs, and others
mentioned. Also, check for any other models exposing Slug and apply the same
validation. Additionally, audit and clean existing data to ensure no Slug values
exceed 450 characters or cause duplicates before applying the migration.
There was a problem hiding this comment.
Code Review
This pull request introduces significant and valuable features, primarily multi-language support and soft deletion for books. The changes are extensive, touching various layers of the application, and demonstrate a good effort towards improving internationalization and data management. Key improvements include:
- Localization: Comprehensive localization using resource files (
.resx) for UI text and validation messages. A newMessageServiceandLanguageControllerfacilitate this. - Soft Deletion: The
Bookentity now supports soft deletion with anIsDeletedflag, and the database schema has been updated accordingly, including a filtered unique index for slugs. - Refactoring: Validation messages in ViewModels are now localized. Hardcoded error messages in services have been replaced with calls to the
MessageService. Namespace typos have been corrected. - Modern C# Features: Use of
requiredmembers in ViewModels. - Database Migrations: Migrations are included to reflect schema changes.
- Code Quality: Introduction of custom exceptions and improved error reporting in some services.
The changes are generally well-implemented. My review focuses on further enhancing correctness, maintainability, and clarity. Please see the specific comments below.
| var exists = await _context.Keywords | ||
| .AnyAsync(k => EF.Functions.Collate(k.Word, "SQL_Latin1_General_CP1_CI_AI") == model.Word, ct); | ||
| if (exists) | ||
| { | ||
| throw new Exception("این کلیدواژه قبلاً ثبت شده است."); | ||
| throw new Exception(_messageService.Get("Keyword_Found")); | ||
| } |
There was a problem hiding this comment.
When a keyword already exists (exists == true), an exception is thrown with the message _messageService.Get("Keyword_Found"). The resource key "Keyword_Found" translates to "Keyword found" (English) or "کلید واژه یافت شد" (Persian), which sounds like a success message rather than an error indicating a duplicate.
Consider using a more appropriate resource key like "Error_KeywordAlreadyExists" with a corresponding message like "This keyword has already been registered." to clearly communicate the error.
| using Refhub.Service.Implement; | ||
| using Refhub.Service.Interface; |
There was a problem hiding this comment.
These using directives for Refhub.Service.Implement and Refhub.Service.Interface do not appear to be used in this file. Consider removing them to keep the code clean and reduce unnecessary dependencies.
| [RegularExpression( | ||
| // At least one letter, one digit, 6-64 chars, allow specials. | ||
| @"^(?=.*[A-Za-z])(?=.*\d).{6,64}$", | ||
| ErrorMessageResourceType = typeof(Messages), | ||
| ErrorMessageResourceName = nameof(Messages.Password_Regex_Invalid))] |
There was a problem hiding this comment.
The regular expression for password validation (^(?=.*[A-Za-z])(?=.*\d).{6,64}$) allows for a length of 6-64 characters and permits any character type (letters, numbers, symbols, spaces) due to the . metacharacter.
However, the associated validation message, Messages.Password_Regex_Invalid, currently states: "Must contain at least one letter, one number and 6 characters." (English) or "باید حداقل یک حرف، یک عدد و ۶ کاراکتر داشته باشد" (Persian).
This message should be updated to accurately reflect all constraints imposed by the regex, including the maximum length (64 characters) and the types of characters allowed (e.g., if special characters or spaces are permitted). This will provide clearer guidance to users.
A similar review should be done for LoginVM.cs which uses a slightly different regex ([\S]{6,64}$ which disallows whitespace) to ensure its message is also accurate.
| @@ -1,18 +1,22 @@ | |||
| using Refhub.Data.Models; | |||
| using Microsoft.IdentityModel.Tokens; | |||
There was a problem hiding this comment.
The using Microsoft.IdentityModel.Tokens; directive does not seem to be used in this file. It's good practice to remove unused usings to improve code clarity and reduce compilation overhead.
| ba.Book.BookAuthors.Select(x => x.Author.FullName)) | ||
| ?? _messageService.Get("Error_NotDefined"), | ||
|
|
There was a problem hiding this comment.
The string.Join(", ", ba.Book.BookAuthors.Select(x => x.Author.FullName)) will return an empty string ("") if ba.Book.BookAuthors is empty or if all FullName properties are null/empty. An empty string is not null, so the null-coalescing operator ?? _messageService.Get("Error_NotDefined") will not be triggered in such cases, and AuthorFullName will be an empty string.
If an empty string is the desired representation for a book with no authors, then the ?? _messageService.Get("Error_NotDefined") part is redundant here. If a specific placeholder (like "N/A" or the localized "Error_NotDefined") is desired when no authors are present, the logic would need adjustment (e.g., checking if the joined string is empty).
| var exists = await _context.Keywords | ||
| .AnyAsync(k => EF.Functions.Collate(k.Word, "SQL_Latin1_General_CP1_CI_AI") == model.Word, ct); |
There was a problem hiding this comment.
Using EF.Functions.Collate(k.Word, "SQL_Latin1_General_CP1_CI_AI") for case-insensitive and accent-insensitive comparison is efficient for SQL Server. However, hardcoding the collation string "SQL_Latin1_General_CP1_CI_AI" makes this part of the query specific to SQL Server and might affect database portability if that's a future concern.
If the application is intended to be SQL Server-specific, this is acceptable. Otherwise, you might explore more database-agnostic ways to achieve case-insensitive comparisons if needed, though ToLower() in LINQ to Entities often translates to LOWER() in SQL which can be less efficient than using collations for indexed columns.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
Refhub/Service/Implement/AuthorService.cs (1)
90-119: 🛠️ Refactor suggestionStill throwing generic
Exception— use domain-specific onesThe new custom exceptions exist but are not used everywhere:
-throw new Exception(_messageService.Get("Error_AuthorNotFound")); +throw new EntityNotFoundException(_messageService.Get("Error_AuthorNotFound"));-throw new Exception(_messageService.Get("Error_SlugExists")); +throw new DuplicateSlugException(_messageService.Get("Error_SlugExists"));Likewise in
DeleteAuthorAsync. This preserves structured error handling and REST status mapping.Also applies to: 128-134
♻️ Duplicate comments (15)
Refhub/Views/Book/List.cshtml (1)
2-2: Same namespace fix – see prior commentThe same remarks about residual references and BOM removal apply here.
Refhub/Components/Views/Home/BestBookView.cshtml (1)
1-1: Same namespace fix – see prior commentThe same remarks about residual references and BOM removal apply here.
Refhub/Views/Book/BookDetails.cshtml (1)
1-1: Same namespace fix – see prior commentThe same remarks about residual references and BOM removal apply here.
Refhub/Components/Views/Home/LastBookView.cshtml (1)
1-1: Same namespace fix – see prior commentThe same remarks about residual references and BOM removal apply here.
Refhub/Data/Configuration/BookConfiguration.cs (1)
15-17: 🛠️ Refactor suggestionStill missing length constraint on
Slug
The previous review already recommended aligning the model with the database by addingHasMaxLength(450)(or whatever the column size actually is). Leaving it unconstrained can yield runtime exceptions when very long slugs are persisted or when EF creates migrations that differ from the real schema.-builder.Property(b => b.Slug).IsRequired(); +builder.Property(b => b.Slug) + .IsRequired() + .HasMaxLength(450); // keep in sync with DB column sizeRefhub/Migrations/20250612110054_AddIsDeletedOnBookTable.cs (1)
13-23: Extraneous blank lines persistThe exact blank lines flagged in the previous review are still present, reducing readability of the migration.
- - - migrationBuilder.AddColumn<bool>( @@ - -Also applies to: 29-36
Refhub/Models/Books/CreateBookVM.cs (1)
38-42: Ambiguous property name – stillAnotherIdPrior reviews already recommended renaming this collection to something meaningful (e.g.,
AuthorIdsorSelectedAuthorIds).
Keeping the vague name hurts readability across the codebase.Refhub/Service/Implement/KeywordService.cs (1)
27-27: Misleading message keyKeyword_FoundThrowing an exception with key “Keyword_Found” when the keyword already exists reads like a success message.
Rename the key to something likeKeyword_AlreadyExistsand update the resource & usages.Refhub/Controllers/LanguageController.cs (2)
12-25: Duplicate logic betweenPersian()andEnglish()Both actions differ only by the culture string. Extract the common code into a private helper or one parameterised action to cut duplication.
3-4: Unusedusingdirectives
Refhub.Service.ImplementandRefhub.Service.Interfaceare not referenced in this file. Remove them to keep the file lean.Refhub/Models/Books/UpdateBookVM.cs (1)
39-41: RenameAnotherIdfor clarityThe collection still carries the non-descriptive name
AnotherId. Favour something likeAuthorIdsto communicate intent.Refhub/Resources/Messages.resx (1)
177-179: Password validation message still inaccuratePrior review suggested reflecting the 6-64 length and letter-digit requirements. Message remains unchanged.
Refhub/Resources/Messages.fa.resx (1)
178-179: 🛠️ Refactor suggestionPassword regex description still incomplete
Prior feedback suggested reflecting the full
6-64length constraint. Current string only mentions “۶ کاراکتر”. Consider the previously-proposed wording:- <value>باید حداقل یک حرف، یک عدد و ۶ کاراکتر داشته باشد</value> + <value>رمز عبور باید بین ۶ تا ۶۴ کاراکتر باشد و حداقل شامل یک حرف و یک عدد باشد.</value>Refhub/Migrations/20250612110054_AddIsDeletedOnBookTable.Designer.cs (1)
289-291: Un-filteredSlugindex noted, but fixed by the very next migrationThis snapshot still shows
.HasIndex("Slug").IsUnique();.
Because a later migration (AddFilterSlugIndexByIsDeleteOnBook) recreates the index with a filter, no change is needed here; just make sure both migrations run in order in production.Refhub/Resources/Messages.Designer.cs (1)
99-115:Book_AnotherId*keys still present – rename toBook_AuthorId*for clarityThis was flagged earlier; the ambiguous “AnotherId” leaks into strongly-typed wrappers and will confuse future contributors.
Rename the keys in both.resxfiles ➜ regenerate ➜ update usages (CreateBookVM,UpdateBookVM, …).
🧹 Nitpick comments (25)
Refhub/Tools/ExtentionMethod/CookieConfigureExtentionMethod.cs (1)
1-1: Namespace typo fixed, but class/file names remain misspelledNice catch on correcting the namespace.
However, the class name (CookieConfigureExtentionMethod) and the file path (ExtentionMethod) still carry the original typo “Extention”. This leaves an odd mix ofExtensionMethodandExtentionMethodthroughout the codebase and IDE look-ups.Consider renaming the class/file to
CookieConfigureExtensionMethodfor consistency and to avoid future confusion.Refhub/Tools/ExtentionMethod/IdentityConfigureExtentionMethod.cs (1)
5-5: Same inconsistency as aboveNamespace corrected, but the class (
IdentityConfigureExtentionMethod) —and the file folder—still use the typo. A unified, typo-free naming convention (IdentityConfigureExtensionMethod) improves discoverability and lowers the odds of import mistakes.Refhub/Tools/ExtentionMethod/PathExtionMethod.cs (1)
3-3: Mixed typos in file/class namesGood namespace fix.
The file is namedPathExtionMethod.cs(missing the second “e”), while the class isPathImageExtionMethod(includes the earlier typo). Recommend renaming both toPathExtensionMethod(or splitting into clearer purpose-specific files) to maintain a clean API surface.Refhub/Tools/ExtentionMethod/ContextConfigureExtentionMethod.cs (1)
4-4: Consistent naming clean-upSame pattern: namespace fixed, but class/file still misspelled (
ContextConfigureExtentionMethod). A one-time rename now prevents a long tail of minor friction later.Refhub/Data/Configuration/BookConfiguration.cs (1)
12-12: Remove stray blank lineThe lone blank line adds noise and breaks the otherwise compact configuration block.
- +Refhub/Tools/ExtentionMethod/AddServiceExtentionMethod.cs (2)
4-8: Typo in class & file name: “Extention” → “Extension”The namespace was fixed, but the class and file name still carry the misspelling. This leaks into symbol names (
AddServiceExtentionMethod) and may confuse consumers.- public static class AddServiceExtentionMethod + public static class AddServiceExtensionMethodRename the file accordingly.
16-17: Lifetime choice forMessageService
MessageServicelooks stateless and merely fetches localized strings. Registering it asSingletonsaves allocations since no request-scoped state is held.-collection.AddScoped<IMessageService, MessageService>(); +collection.AddSingleton<IMessageService, MessageService>();Refhub/Service/Interface/IMessageService.cs (1)
3-7: Optional: add XML docs & async overload for consistency.Most service interfaces in the codebase expose XML documentation and sometimes asynchronous members. Adding summaries and an
Task<string> GetAsync(...)overload would improve discoverability and future-proof long-running look-ups (e.g., remote stores).Purely optional – interface is otherwise clean.
Refhub/Migrations/AppDbContextModelSnapshot.cs (1)
264-266: Add explicit default forIsDeletedin the model snapshotThe migration that introduced
IsDeletedsetsdefaultValue: false, but the model snapshot omits.HasDefaultValue(false).
Leaving it out is functionally fine today, yet it means:
- A future
Add-Migrationcan detect this as a change and try to drop/re-add the default constraint.- The intent of the column (soft-delete defaulting to false) is less obvious when reading the snapshot.
Consider persisting the default to keep the model stable:
- b.Property<bool>("IsDeleted") - .HasColumnType("bit"); + b.Property<bool>("IsDeleted") + .HasColumnType("bit") + .HasDefaultValue(false);Refhub/Models/Category/UpdateCategoryVM.cs (2)
9-13: Resource key naming nit
Name_Max_50follows a different prefix scheme fromCategory_NameRequired.
For consistency across messages (and easier look-ups), consider either:•
Category_Name_Max_50
or
•Name_Max_50+ rename the required key toName_RequiredNot a blocker, just maintainability polish.
14-18: Uniform casing between domain & VMEntity model:
Category.slug(lower-case)
View-model:Slug(PascalCase)AutoMapper will map fine, but the mixed casing leaks into LINQ
nameof()usage and string-based reflections. Aligning both sides (preferSlug) avoids subtle bugs and reduces mental overhead.Refhub/Models/Category/CreateCategoryVM.cs (1)
8-22: Align casing & message-key convention (same asUpdateCategoryVM)Same remarks as in
UpdateCategoryVM:
- Consider renaming keys for uniformity (
Category_prefix or none).- Harmonise the
Slug/slugproperty name with the domain model.Otherwise the validation wiring looks solid.
Refhub/Models/Books/CreateBookVM.cs (1)
18-21: Message key does not match validation intent
[Range]validates the value is out of range, not that it is “required”.
The resource keyBook_PageCountRequiredsuggests a missing value.
Consider introducing a separate key such asBook_PageCountInvalid(or renaming the existing one) so users see an accurate error.Refhub/Models/Books/UpdateBookVM.cs (1)
22-25: Same mismatch between[Range]and “…Required” messageAs in
CreateBookVM, a value that fails a range check is not “missing”. Align the resource key wording to avoid confusing users.Refhub/Service/Implement/MessageService.cs (1)
12-16: Prefer typedIStringLocalizer<Messages>over manual factory lookupYou can inject
IStringLocalizer<Messages>directly and drop the reflection + factory code.
This removes the brittle"Messages"string literal and assembly name guesswork while keeping the same runtime behaviour.- private readonly IStringLocalizer _localizer; - - public MessageService(IStringLocalizerFactory factory) - { - var assemblyName = Assembly.GetExecutingAssembly().GetName().Name ?? "Refhub"; - _localizer = factory.Create(nameof(Messages), assemblyName); - } + private readonly IStringLocalizer<Messages> _localizer; + + public MessageService(IStringLocalizer<Messages> localizer) + { + _localizer = localizer; + }Refhub/Resources/Messages.fa.resx (1)
214-214: Leading space before sentence
Book_SlugMaxLengthstarts with an extra space which will surface in UI.- <value> طول عنوان در مرورگر حداکثر 450 حرف باشد</value> + <value>طول عنوان در مرورگر حداکثر 450 حرف باشد</value>Refhub/Tools/Exceptions/DuplicateSlugException.cs (1)
1-24: Add serialization & separate files for exception typesFor completeness and to enable cross-app-domain scenarios, mark exceptions as
[Serializable]and add the protected(SerializationInfo, StreamingContext)constructor.
Also consider movingEntityNotFoundExceptionto its own file to keep 1-type-per-file convention.+using System.Runtime.Serialization; ... -[public] class DuplicateSlugException : Exception +[Serializable] +public sealed class DuplicateSlugException : Exception { public DuplicateSlugException(string message) : base(message) { } public DuplicateSlugException(string message, Exception inner) : base(message, inner) { } + protected DuplicateSlugException(SerializationInfo info, StreamingContext ctx) : base(info, ctx) { } }(similar changes for
EntityNotFoundException)Refhub/Service/Implement/AuthorService.cs (1)
1-2: Unused dependency import
Microsoft.IdentityModel.Tokensis not referenced in this file.-using Microsoft.IdentityModel.Tokens; +// (removed unused using)Refhub/Service/Implement/UserService.cs (1)
67-71: Successful path returns an empty view-model
AddToRoleForUserInAdminPanelAsyncreturns a blankUserListAdminVM, losing context for callers (id, role state, etc.).
Populate at least theIdandIsAdminfields or return the updated list.Refhub/Tools/ExtentionMethod/MultiLanguageExtentionMethod.cs (1)
31-40: Register localization services explicitly for robustness
AddViewLocalizationworks even ifAddLocalizationwasn’t called, but only because it silently adds it.
Being explicit in the DI setup is clearer and future-proof:-public static IMvcBuilder AddMultiLanguage(this IMvcBuilder mvc) -{ - mvc.AddViewLocalization(LanguageViewLocationExpanderFormat.Suffix, option => - { - option.ResourcesPath = "Resources"; - }).AddDataAnnotationsLocalization(); - - return mvc; -} +public static IMvcBuilder AddMultiLanguage(this IMvcBuilder mvc) +{ + mvc.Services.AddLocalization(opts => opts.ResourcesPath = "Resources"); + + mvc.AddViewLocalization(LanguageViewLocationExpanderFormat.Suffix) + .AddDataAnnotationsLocalization(); + + return mvc; +}Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.Designer.cs (1)
289-292: Use SQL-Server quoting for the filtered indexOther filters in the snapshot quote columns (
[NormalizedName] IS NOT NULL).
For consistency (and to avoid edge-case name collisions) quoteIsDeletedtoo:- .HasFilter("IsDeleted = 0"); + .HasFilter("[IsDeleted] = 0");Refhub/Models/Users/LoginVM.cs (1)
20-23: Consider stronger minimum password lengthA 6-character minimum is quite low by today’s standards. Industry guidance (e.g. NIST) recommends at least 8 characters.
If policy allows, update the regex and translation strings to enforce 8+ characters.
Refhub/Resources/Messages.Designer.cs (3)
135-142: Grammar nit – “Select a Image” → “Select an image”The English article is incorrect and “Image” should be lowercase for consistency.
Fix in the.resxfiles then regenerate this designer.
324-331: Minor punctuation – stray space before full stop
"Remember Me! ."has an extra space. Clean up in the resource file and regenerate.
1-26: Confirm committing auto-generated designer is intentionalDesigner files can be regenerated; committing them increases diff noise and merge conflicts.
Consider excluding*.Designer.csfrom source control and relying on CI build output unless you have a specific reason to keep them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
Refhub/Areas/Admin/Views/ManageBook/Index.cshtml(1 hunks)Refhub/Areas/Admin/Views/ManageBook/Update.cshtml(1 hunks)Refhub/Components/Views/Home/BestBookView.cshtml(1 hunks)Refhub/Components/Views/Home/LastBookView.cshtml(1 hunks)Refhub/Controllers/LanguageController.cs(1 hunks)Refhub/Data/Configuration/BookConfiguration.cs(1 hunks)Refhub/Data/Models/Book.cs(1 hunks)Refhub/Migrations/20250612083534_AddUniqueIndexToBookSlugAndModifyType.Designer.cs(1 hunks)Refhub/Migrations/20250612083534_AddUniqueIndexToBookSlugAndModifyType.cs(1 hunks)Refhub/Migrations/20250612110054_AddIsDeletedOnBookTable.Designer.cs(1 hunks)Refhub/Migrations/20250612110054_AddIsDeletedOnBookTable.cs(1 hunks)Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.Designer.cs(1 hunks)Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.cs(1 hunks)Refhub/Migrations/AppDbContextModelSnapshot.cs(3 hunks)Refhub/Models/Authors/AuthorVM.cs(1 hunks)Refhub/Models/Books/CreateBookVM.cs(1 hunks)Refhub/Models/Books/UpdateBookVM.cs(2 hunks)Refhub/Models/Category/CreateCategoryVM.cs(1 hunks)Refhub/Models/Category/UpdateCategoryVM.cs(1 hunks)Refhub/Models/Keywords/CreateKeywordVM.cs(1 hunks)Refhub/Models/Keywords/EditKeywordVM.cs(1 hunks)Refhub/Models/Users/LoginVM.cs(1 hunks)Refhub/Models/Users/RegisterVM.cs(1 hunks)Refhub/Program.cs(3 hunks)Refhub/Refhub.csproj(1 hunks)Refhub/Resources/Messages.Designer.cs(1 hunks)Refhub/Resources/Messages.fa.resx(1 hunks)Refhub/Resources/Messages.resx(1 hunks)Refhub/Service/Implement/AuthorService.cs(5 hunks)Refhub/Service/Implement/KeywordService.cs(1 hunks)Refhub/Service/Implement/MessageService.cs(1 hunks)Refhub/Service/Implement/UserService.cs(4 hunks)Refhub/Service/Interface/IMessageService.cs(1 hunks)Refhub/Tools/Exceptions/DuplicateSlugException.cs(1 hunks)Refhub/Tools/ExtentionMethod/AddServiceExtentionMethod.cs(2 hunks)Refhub/Tools/ExtentionMethod/ContextConfigureExtentionMethod.cs(1 hunks)Refhub/Tools/ExtentionMethod/CookieConfigureExtentionMethod.cs(1 hunks)Refhub/Tools/ExtentionMethod/IdentityConfigureExtentionMethod.cs(1 hunks)Refhub/Tools/ExtentionMethod/MultiLanguageExtentionMethod.cs(1 hunks)Refhub/Tools/ExtentionMethod/PathExtionMethod.cs(1 hunks)Refhub/Tools/Static/AuthorMessage.cs(0 hunks)Refhub/Tools/Static/ErrorMessageAuthenticationStatic.cs(0 hunks)Refhub/Views/Book/BookDetails.cshtml(1 hunks)Refhub/Views/Book/List.cshtml(1 hunks)
💤 Files with no reviewable changes (2)
- Refhub/Tools/Static/ErrorMessageAuthenticationStatic.cs
- Refhub/Tools/Static/AuthorMessage.cs
🧰 Additional context used
🧬 Code Graph Analysis (7)
Refhub/Migrations/20250612110054_AddIsDeletedOnBookTable.cs (1)
Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.cs (2)
Up(11-23)Down(26-37)
Refhub/Data/Configuration/BookConfiguration.cs (1)
Refhub/Data/Models/Book.cs (1)
Book(3-27)
Refhub/Models/Category/CreateCategoryVM.cs (1)
Refhub/Data/Models/Category.cs (1)
Category(3-12)
Refhub/Models/Category/UpdateCategoryVM.cs (1)
Refhub/Data/Models/Category.cs (1)
Category(3-12)
Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.cs (1)
Refhub/Migrations/20250612110054_AddIsDeletedOnBookTable.cs (2)
Up(11-24)Down(27-38)
Refhub/Service/Implement/KeywordService.cs (1)
Refhub/Models/Keywords/CreateKeywordVM.cs (1)
CreateKeywordVM(6-11)
Refhub/Service/Implement/AuthorService.cs (2)
Refhub/Data/Models/Book.cs (1)
Book(3-27)Refhub/Data/Models/Author.cs (1)
Author(3-12)
🔇 Additional comments (15)
Refhub/Areas/Admin/Views/ManageBook/Index.cshtml (1)
2-2: Namespace import fixed – verify other Razor viewsThis view now imports the corrected namespace. Make sure every Razor view updated in this PR—or previously—also uses
Refhub.Tools.ExtensionMethod; otherwise extension methods likeConvertForBookPathImage()will fail to resolve.Refhub/Areas/Admin/Views/ManageBook/Update.cshtml (1)
1-1:Details
✅ Verification successful
Namespace typo fixed – verify & clean up
Good catch changing
ExtentionMethod➜ExtensionMethod, this will unblock the build.
Run the quick scan below to be sure no stray references remain (CI was red):
There’s a UTF-8 BOM in this line (
). It is harmless but noisy in diffs; consider saving the file without BOM:-@using Refhub.Tools.ExtensionMethod +@using Refhub.Tools.ExtensionMethod
🏁 Script executed:
#!/bin/bash # Find any forgotten usages of the old miss-spelled namespace rg --line-number "Refhub\\.Tools\\.ExtentionMethod" || trueLength of output: 198
🏁 Script executed:
#!/bin/bash # Re-run scan for any forgotten usages of the old misspelled namespace rg --line-number "Refhub\\.Tools\\.ExtentionMethod" || trueLength of output: 58
#!/bin/bash # Check for UTF-8 BOM in Update.cshtml grep -n $'\xEF\xBB\xBF' Refhub/Areas/Admin/Views/ManageBook/Update.cshtml || true
Namespace typo fixed; no stray references detected, remove BOM for cleaner diffs
All occurrences of the old misspelled namespace
Refhub.Tools.ExtentionMethodhave been cleaned up—no further matches found in the codebase. Please remove the UTF-8 BOM from the top of the file to avoid noisy diffs:• Refhub/Areas/Admin/Views/ManageBook/Update.cshtml
-@using Refhub.Tools.ExtensionMethod +@using Refhub.Tools.ExtensionMethodRefhub/Data/Models/Book.cs (1)
17-17: Add global-query filter or guard for soft-delete flagHaving an
IsDeletedflag alone does not prevent accidentally selecting deleted rows.
Unless theDbContextconfigures a global filter (modelBuilder.Entity<Book>().HasQueryFilter(b => !b.IsDeleted)), every query must remember to addWhere(b => !b.IsDeleted). That is error-prone and defeats the purpose of soft deletion.Do we already have such a filter in
AppDbContext? If not, please add it or confirm that every repository/service layer explicitly filters onIsDeleted = false.Refhub/Models/Authors/AuthorVM.cs (1)
8-15: Good shift to resource-based validation messages
The newRequiredattributes correctly reference theMessagesresource class—nice localisation upgrade.Refhub/Models/Keywords/CreateKeywordVM.cs (1)
8-10: Validation message successfully localized – looks good.
Switching the[Required]attribute to useMessages.Keyword_Requiredcleanly aligns this VM with your new localization infrastructure. No functional or stylistic issues spotted here.Refhub/Program.cs (1)
14-16: Verify extension-method targets & middleware ordering.
AddMultiLanguage()is chained offAddControllersWithViews(). Ensure the extension is defined forIMvcBuilder; otherwise the project won’t compile.app.UseMultiLanguage()runs afterUseStaticFiles().UseRequestLocalization(whichUseMultiLanguagepresumably wraps) is normally placed before static-file handling so culture-specific static assets (e.g., “wwwroot/fa/…”) resolve correctly. Moving it directly afterUseHttpsRedirection()is the common pattern.Please double-check both points before merging.
Also applies to: 52-53
Refhub/Models/Keywords/EditKeywordVM.cs (1)
10-12: Localized validation applied consistently – good.
The change mirrorsCreateKeywordVM, keeping the VMs aligned with the new resource-based messages.Refhub/Migrations/AppDbContextModelSnapshot.cs (1)
286-288: Confirm the filter is mirrored inBookConfigurationto avoid churnThe snapshot uses
HasFilter("IsDeleted = 0").
If the fluent configuration class still contains the old, un-filtered index, EF will keep generating dangling “rename” migrations. Please verify thatRefhub/Data/Configuration/BookConfiguration.cs(orOnModelCreating) contains exactly the same filter string.Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.cs (1)
13-23:Details
❓ Verification inconclusive
Migration may fail on existing data – check for collisions first
If any non-deleted rows share the same slug,
CreateIndex … unique: true, filter: "IsDeleted = 0"will throw.
Run a quick check before applying to prod/staging.Replace the DB client with the one used in your environment.
🏁 Script executed:
#!/bin/bash # Detect duplicate slugs among non-deleted books sqlite3 ? # replace with sqlcmd/psql if needed -- SELECT Slug, COUNT(*) c FROM Books WHERE IsDeleted = 0 GROUP BY Slug HAVING c > 1;Length of output: 380
Ensure no duplicate non-deleted slugs before applying the migration
The new unique, filtered index on
Books.Slug(IsDeleted = 0) will fail if any two non-deleted rows share the same slug. Please verify your data with the appropriate database client before running this migration. For example:• SQL Server (using
sqlcmd):sqlcmd -S <SERVER> -d <DATABASE> -Q "SET NOCOUNT ON; \ SELECT Slug, COUNT(*) AS DuplicateCount \ FROM Books \ WHERE IsDeleted = 0 \ GROUP BY Slug \ HAVING COUNT(*) > 1;"• PostgreSQL (using
psql):psql "host=<HOST> port=<PORT> dbname=<DB> user=<USER>" \ -c "SELECT \"Slug\", COUNT(*) AS duplicate_count \ FROM \"Books\" \ WHERE NOT \"IsDeleted\" \ GROUP BY \"Slug\" \ HAVING COUNT(*) > 1;"If any rows are returned, resolve or dedupe those slugs before applying the index.
Refhub/Migrations/20250612083534_AddUniqueIndexToBookSlugAndModifyType.cs (1)
13-25:Details
❓ Verification inconclusive
Guard against null / oversize slugs before altering the column
AlterColumn … nullable: false, type: nvarchar(450)assumes:
- No NULL in
Books.Slug- All values ≤ 450 chars
If either assumption is violated, the migration breaks mid-flight.
Clean / truncate offending rows or decide on a fallback slug generator before running the migration.
🏁 Script executed:
#!/bin/bash # 1. Null slugs sqlcmd -Q "SELECT COUNT(*) FROM Books WHERE Slug IS NULL" # 2. Over-length slugs sqlcmd -Q "SELECT TOP 10 Id, LEN(Slug) AS L FROM Books WHERE LEN(Slug) > 450"Length of output: 333
Ensure no null or oversized Slug values before running this migration
- File: Refhub/Migrations/20250612083534_AddUniqueIndexToBookSlugAndModifyType.cs (lines 13–25)
- Changing
Slugtonullable: falseandnvarchar(450)will fail if any existing Slug is NULL or longer than 450 characters.Please verify and clean/truncate—or generate fallback slugs—prior to applying this migration. For example, run against your database:
SELECT COUNT(*) FROM Books WHERE Slug IS NULL; SELECT TOP 10 Id, LEN(Slug) AS Length FROM Books WHERE LEN(Slug) > 450;Refhub/Service/Implement/KeywordService.cs (1)
23-25: Collation-specific compare may bypass index if the parameter collation differsUsing
EF.Functions.Collateonly on the column side keeps the index sargable if the underlying column shares the same collation.
Double-check thatKeywords.Wordis actuallySQL_Latin1_General_CP1_CI_AI; otherwise the query will force a runtime collation conversion and still miss the index.Refhub/Service/Implement/MessageService.cs (1)
24-28: Minor: avoid double evaluation ofparamsoverload
_localizer[key, parameters]internally formats, sovalue.ResourceNotFoundis sufficient without the secondvalue.Valueevaluation.No code change required – mention for awareness only.
Refhub/Service/Implement/UserService.cs (1)
55-56:Error.NotFoundoverload mismatchTwo-parameter overload is used above (
"User.NotFound", ...).
For consistency and clarity, supply an explicit error code here too.-return Error.NotFound(_messageService.Get("Account_LoginInvalid")); +return Error.NotFound("Account.LoginInvalid", _messageService.Get("Account_LoginInvalid"));Refhub/Migrations/20250612083534_AddUniqueIndexToBookSlugAndModifyType.Designer.cs (1)
286-288: No action – superseded by later filtered-index migrationThis unique index is intentionally replaced in the subsequent
AddFilterSlugIndexByIsDeleteOnBookmigration.
Ensure DBA applies migrations in sequence to avoid transient duplicate-index warnings.Refhub/Models/Users/RegisterVM.cs (1)
8-14: Validation attributes look goodUsing resource-based messages and the
requiredmodifier is clean and compile-time safe.
| <ItemGroup> | ||
| <EmbeddedResource Update="Resources\Messages.resx"> | ||
| <Generator>PublicResXFileCodeGenerator</Generator> | ||
| <LastGenOutput>Messages.Designer.cs</LastGenOutput> | ||
| </EmbeddedResource> | ||
| <Compile Update="Resources\Messages.Designer.cs"> | ||
| <DesignTime>True</DesignTime> | ||
| <AutoGen>True</AutoGen> | ||
| <DependentUpon>Messages.resx</DependentUpon> | ||
| </Compile> | ||
| </ItemGroup> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Resource generation may break on non-Windows agents.
PublicResXFileCodeGenerator is a Visual-Studio generator unavailable on Linux/macOS CI runners. On cross-platform builds the designer file won’t be produced, causing missing-type compilation errors.
Consider dropping the custom generator and rely on the SDK’s built-in ResX handling, or switch to ResXFileCodeGenerator, e.g.:
- <Generator>PublicResXFileCodeGenerator</Generator>
+ <Generator>ResXFileCodeGenerator</Generator>Also, replace back-slashes with forward-slashes (Resources/Messages.resx) for OS-agnostic paths.
🤖 Prompt for AI Agents
In Refhub/Refhub.csproj around lines 26 to 36, the use of
PublicResXFileCodeGenerator and backslashes in resource paths causes issues on
non-Windows CI agents. Replace PublicResXFileCodeGenerator with
ResXFileCodeGenerator to ensure cross-platform compatibility, and change all
backslashes in file paths to forward slashes to make paths OS-agnostic.
Refhub/Resources/Messages.resx
Outdated
| <data name="Keyword_Found" xml:space="preserve"> | ||
| <value>Keyword found</value> | ||
| </data> |
There was a problem hiding this comment.
Keyword_Found wording implies success, not duplication
For duplicate-keyword errors the value “Keyword found” is counter-intuitive.
Rename key/value to Keyword_AlreadyExists / “This keyword already exists.” and update all references.
- <data name="Keyword_Found" xml:space="preserve">
- <value>Keyword found</value>
+ <data name="Keyword_AlreadyExists" xml:space="preserve">
+ <value>This keyword already exists.</value>
</data>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <data name="Keyword_Found" xml:space="preserve"> | |
| <value>Keyword found</value> | |
| </data> | |
| <data name="Keyword_AlreadyExists" xml:space="preserve"> | |
| <value>This keyword already exists.</value> | |
| </data> |
🤖 Prompt for AI Agents
In Refhub/Resources/Messages.resx around lines 210 to 212, the message key
"Keyword_Found" and its value "Keyword found" imply success rather than
indicating a duplicate keyword error. Rename the key to "Keyword_AlreadyExists"
and update the value to "This keyword already exists." Then, search the entire
codebase for all references to "Keyword_Found" and update them to use the new
key "Keyword_AlreadyExists" to maintain consistency.
| [RegularExpression( | ||
| // At least one letter, one digit, 6-64 chars, allow specials. | ||
| @"^(?=.*[A-Za-z])(?=.*\d).{6,64}$", | ||
| ErrorMessageResourceType = typeof(Messages), | ||
| ErrorMessageResourceName = nameof(Messages.Password_Regex_Invalid))] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten password regex – it currently allows whitespace and control chars
The . wildcard matches any character except \n, so spaces, tabs, and other non-printable symbols are accepted.
If that’s not intentional, restrict the character class, e.g.:
-@"^(?=.*[A-Za-z])(?=.*\d).{6,64}$"
+@"^(?=.*[A-Za-z])(?=.*\d)[^\s]{6,64}$"This keeps the ≥1 letter/≥1 digit rule while disallowing whitespace.
Update the corresponding resource message if the semantics change.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [RegularExpression( | |
| // At least one letter, one digit, 6-64 chars, allow specials. | |
| @"^(?=.*[A-Za-z])(?=.*\d).{6,64}$", | |
| ErrorMessageResourceType = typeof(Messages), | |
| ErrorMessageResourceName = nameof(Messages.Password_Regex_Invalid))] | |
| [RegularExpression( | |
| // At least one letter, one digit, 6-64 chars, disallow whitespace. | |
| @"^(?=.*[A-Za-z])(?=.*\d)[^\s]{6,64}$", | |
| ErrorMessageResourceType = typeof(Messages), | |
| ErrorMessageResourceName = nameof(Messages.Password_Regex_Invalid))] |
🤖 Prompt for AI Agents
In Refhub/Models/Users/RegisterVM.cs around lines 19 to 23, the password regex
currently allows whitespace and control characters due to the use of the '.'
wildcard. To fix this, replace the '.' with a character class that excludes
whitespace and control characters but still enforces at least one letter and one
digit with length 6 to 64. Also, update the error message resource if the
validation semantics change.
|
@abolfazlshs80 please read the rabbit code comment for the last commit and fix them. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements by implementing multi-language support, soft deletion for books, and enhanced validation and error handling. The localization of messages and validation attributes is a great step towards a more user-friendly and maintainable application. The introduction of soft deletion with filtered unique indexes is a robust solution for managing book records. Password validation rules have also been strengthened, contributing to better security. However, a potential issue was identified where the ImagePath property for BookVM is no longer populated in the AuthorService, which could lead to broken image displays in the UI.
| AuthorFullName = string.Join(", ", ba.Book.BookAuthors | ||
| .Select(x => x.Author.FullName) | ||
| .Where(n => !string.IsNullOrWhiteSpace(n))) | ||
| .Trim() switch | ||
| { | ||
| "" => _messageService.Get("Error_NotDefined"), | ||
| var s => s | ||
| }, | ||
|
|
||
| }).ToList(); |
There was a problem hiding this comment.
The ImagePath property for BookVM is no longer being populated in this projection. This will likely lead to broken image displays in views that rely on BookVM from this service, such as Refhub/Views/Book/List.cshtml.
Please ensure that ImagePath is included in the BookVM projection.
Title = ba.Book.Title,
ImagePath = ba.Book.ImagePath,
AuthorFullName = string.Join(", ", ba.Book.BookAuthors
.Select(x => x.Author.FullName)
.Where(n => !string.IsNullOrWhiteSpace(n)))
.Trim() switch
{
"" => _messageService.Get("Error_NotDefined"),
var s => s
},
There was a problem hiding this comment.
Pull Request Overview
This PR refactors extension method namespaces, introduces a centralized IMessageService backed by resource files, and implements soft deletion for books with filtered unique slug indexes.
- Renamed
ExtentionMethodnamespaces toExtensionMethodand updated view imports. - Removed static error message classes; added
MessageServiceand wired it into existing services for localized messages. - Added
IsDeletedto theBookmodel, updated migrations and indexes for soft deletion.
Reviewed Changes
Copilot reviewed 40 out of 44 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Refhub/Tools/ExtentionMethod/MultiLanguageExtentionMethod.cs | New extension methods to configure localization middleware and MVC. |
| Refhub/Service/Implement/MessageService.cs | Implements IMessageService using IStringLocalizer for resource-based messages. |
| Refhub/Service/Implement/UserService.cs | Injects and uses IMessageService for all error messages. |
| Refhub/Data/Models/Book.cs | Added IsDeleted property for soft deletion and default value. |
| Refhub/Migrations/… | Created migrations to add IsDeleted column and filter slug indexes. |
Files not reviewed (4)
- Refhub/Migrations/20250612083534_AddUniqueIndexToBookSlugAndModifyType.Designer.cs: Language not supported
- Refhub/Migrations/20250612110054_AddIsDeletedOnBookTable.Designer.cs: Language not supported
- Refhub/Migrations/20250612110603_AddFilterSlugIndexByIsDeleteOnBook.Designer.cs: Language not supported
- Refhub/Resources/Messages.Designer.cs: Language not supported
|
|
||
| } | ||
|
|
||
| public class EntityNotFoundException : Exception |
There was a problem hiding this comment.
The file DuplicateSlugException.cs declares both DuplicateSlugException and EntityNotFoundException. Consider splitting each exception into its own file to match the class name and improve clarity.
| using Microsoft.IdentityModel.Tokens; | ||
| using Refhub.Data.Models; | ||
| using Refhub.Models.Authors; | ||
| using Refhub.Models.Books; |
There was a problem hiding this comment.
The Microsoft.IdentityModel.Tokens using is unused in this file. Removing it will clean up dependencies and avoid confusion.
| using Microsoft.IdentityModel.Tokens; | |
| using Refhub.Data.Models; | |
| using Refhub.Models.Authors; | |
| using Refhub.Models.Books; | |
| using Refhub.Data.Models; | |
| using Refhub.Models.Authors; | |
| using Refhub.Models.Books; | |
| using Refhub.Models.Books; |
| using Refhub.Service.Implement; | ||
| using Refhub.Service.Interface; |
There was a problem hiding this comment.
Neither Refhub.Service.Implement nor Refhub.Service.Interface are used in this controller. Consider removing these unused usings.
| using Refhub.Service.Implement; | |
| using Refhub.Service.Interface; | |
| // Removed unused using directives for Refhub.Service.Implement and Refhub.Service.Interface |
| if (exists) | ||
| { | ||
| throw new Exception("این کلیدواژه قبلاً ثبت شده است."); | ||
| throw new Exception(_messageService.Get("Keyword_AlreadyExists")); |
There was a problem hiding this comment.
Throwing a generic Exception for a business rule can obscure intent. Consider creating and throwing a more specific exception (e.g., DuplicateKeywordException).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
232-239: Update the commentary block – “Automatic PR comments” no longer existsThe explanatory notes still list “✅ Automatic PR comments”, but the
status-reportjob that provided this feature has been removed in this PR. Keeping the note will confuse future maintainers.Trim or re-phrase the bullet list to reflect the current workflow steps.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 228-228: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: 🐳 Docker Build & Test
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
20-25: Verify .NET version consistency
DOTNET_VERSIONis set to9.x.x, while:
- The build-summary message (lines 93-94) claims the target framework is net8.0.
- The Docker summary (line 159) reports the base image
mcr.microsoft.com/dotnet/aspnet:9.0.Please double-check that the intended SDK/runtime versions line up across build, publish, and Docker image or CI failures will follow.
Also applies to: 93-94, 158-160
| *This comment was automatically generated by the build pipeline* 🤖` | ||
| }) | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove trailing whitespace – it breaks YAML linting
yamllint is currently failing on this blank line because it contains stray spaces.
Strip the spaces or delete the line altogether.
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 228-228: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/ci.yml at line 228, there is trailing whitespace on an
otherwise blank line that causes yamllint to fail. Remove all spaces on this
line or delete the line entirely to fix the YAML linting error.
…ew model with title and image path
3 participants
Summary by CodeRabbit
New Features
Improvements
Database Changes
Chores