Avoid cloning the AutoPart pattern in the localized content #15810

hishamco · 2024-04-23T05:56:17Z

Summary by CodeRabbit

New Features
- Enhanced content localization by automatically regenerating permalinks when localizing content.
- Expanded service configurations to support new localization features across content culture pickers and sitemaps.
Refactor
- Introduced new handlers and services for improved content localization management.
Documentation
- Updated project references to support new localization functionalities.

hishamco · 2024-04-23T05:57:16Z

There's might be a better solution, but this the quick one that I go with, @MichaelPetrinolis could you please try the PR

MichaelPetrinolis

If adding a dependency to Autoroute Core is ok, clearing the autoroutepart path does the job. Either it is draftable or not, the localized version has empty path and the user is able to update eg. the localized title and create a localized route. Thanx!

hishamco · 2024-04-23T09:20:57Z

I tried to avoid depending on OC.Autoroute module, but still looking for a better solution if there's

/cc @Skrypt

src/OrchardCore.Modules/OrchardCore.ContentLocalization/DefaultContentLocalizationManager.cs

hishamco · 2024-04-24T01:17:14Z

@coderabbitai review

coderabbitai · 2024-04-24T01:17:22Z

Walkthrough

The update involves improving the content localization process in OrchardCore by automatically regenerating permalinks when content is localized. This enhancement ensures that the autoroute path is correctly handled during localization, addressing a specific issue related to culture names in the path pattern.

Changes

File Path	Change Summary
`.../DefaultContentLocalizationManager.cs`	Added logic to clear `AutoroutePart` path for permalink regeneration during content localization.
`.../OrchardCore.ContentLocalization.csproj`	Added a project reference to `OrchardCore.Autoroute.Core.csproj` to support the new localization functionality.
`.../AutoPartContentLocalizationHandler.cs`	Introduced a new class `AutoPartContentLocalizationHandler` implementing `IContentLocalizationHandler` methods.
`.../Startup.cs`	Various changes including namespace additions and method modifications to support content localization features.

Assessment against linked issues

Objective	Addressed	Explanation
Content Localization Autoroute problem with culture name in path (#15805)	✅	The changes address the issue by enhancing the content localization process to handle autoroute paths correctly.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

hishamco · 2024-04-24T01:20:49Z

Seems coderabbitai ignores my PR :)

Piedone · 2024-04-24T17:29:01Z

Hmm, it seems it didn't actually do a review. I've asked the CR team about this.

hishamco · 2024-05-01T22:06:12Z

@MichaelPetrinolis the new update might much better

coderabbitai

Actionable comments posted: 0

Out of diff range and nitpick comments (1)

src/OrchardCore.Modules/OrchardCore.ContentLocalization/Handlers/AutoPartContentLocalizationHandler.cs (1)

11-19: The logic to clear the AutoroutePart path is correctly implemented to support automatic permalink regeneration.

Consider adding a comment explaining why the path is set to null, for future maintainability.

...dCore.Modules/OrchardCore.ContentLocalization/Handlers/AutoPartContentLocalizationHandler.cs

Co-authored-by: Zoltán Lehóczky <zoltan.lehoczky@lombiq.com>

src/OrchardCore.Modules/OrchardCore.Autoroute/Handlers/AutorouteContentLocalizationHandler.cs

Piedone · 2024-05-02T18:07:30Z

src/OrchardCore.Modules/OrchardCore.Autoroute/Handlers/AutorouteContentLocalizationHandler.cs

This doesn't fix the issue:

2024-05-02_20h06_41.mp4

Please try it without your last suggestion - Using Alter - I'm 100% percentage sure it was working as expected

This is weird I don't know why the

context.ContentItem.Alter<AutoroutePart>(p => p.Path = null);

doesn't work as expected here?!! Do you have any idea or shall I revert the last change

That's good, but please make it work in a statically typed way. We don't need dynamic for this.

I don't know, sorry, I'd also need to get into debugging this to figure it out.

We might need to revise this, using CloningAsync() might resolve the issue but not make sense while LocalizingAsync() should be the suited event for that

if that is the case, then call _session.SaveAsync() after calling LocalizingAsync. But honestly, this sounds like a bad idea to me. IContentHandler should be the right event to hook into at that point. LocalizingAsync should not be used to alter the content item.

LocalizingAsync should not be used to alter the content item.

For that I alert the autoroute path without using Alter() just to solve the above issue

Can you create a test case for the Alter method to make sure STJ isn't causing a problem? Alter should work.

This is #15714 a recent PR that I merged, the Alter was working fine

Avoid cloning the AutoPart pattern in the localized content

a491b55

MichaelPetrinolis approved these changes Apr 23, 2024

View reviewed changes

Piedone requested changes Apr 23, 2024

View reviewed changes

src/OrchardCore.Modules/OrchardCore.ContentLocalization/DefaultContentLocalizationManager.cs Outdated Show resolved Hide resolved

Piedone mentioned this pull request Apr 24, 2024

Set up CodeRabbit for AI code reviews (Lombiq Technologies: OCORE-159) #15813

Merged

This was referenced May 1, 2024

Monthly community metrics report for 2024-04-01..2024-04-30 #15929

Open

Monthly community metrics report for 2024-04-01..2024-04-30 Emmanjr/OrchardCore#2

Open

hishamco added 3 commits May 2, 2024 00:18

Merge branch 'main' into hishamco/issue-15805

77d190b

Address feedback

4e8b6cb

Merge branch 'main' into hishamco/issue-15805

9abfd72

coderabbitai bot reviewed May 1, 2024

View reviewed changes

Merge branch 'main' into hishamco/issue-15805

96a224d

hishamco requested a review from Piedone May 2, 2024 10:49

Piedone requested changes May 2, 2024

View reviewed changes

hishamco and others added 3 commits May 2, 2024 19:38

Apply suggestions from code review

69793d2

Co-authored-by: Zoltán Lehóczky <zoltan.lehoczky@lombiq.com>

Merge branch 'main' into hishamco/issue-15805

143ec74

Move AutorouteContentLocalizationHandler to Autoroute module

2f6f73b

hishamco commented May 2, 2024

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Autoroute/Handlers/AutorouteContentLocalizationHandler.cs Show resolved Hide resolved

hishamco requested a review from Piedone May 2, 2024 16:49

Piedone requested changes May 2, 2024

View reviewed changes

src/OrchardCore.Modules/OrchardCore.Autoroute/Handlers/AutorouteContentLocalizationHandler.cs Outdated Show resolved Hide resolved

Address feedback

11280d1

hishamco requested a review from Piedone May 2, 2024 17:42

Merge branch 'main' into hishamco/issue-15805

62cc6b1

Piedone requested changes May 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid cloning the AutoPart pattern in the localized content #15810

Avoid cloning the AutoPart pattern in the localized content #15810

hishamco commented Apr 23, 2024 •

edited by coderabbitai bot

hishamco commented Apr 23, 2024

MichaelPetrinolis left a comment •

edited

hishamco commented Apr 23, 2024

hishamco commented Apr 24, 2024

coderabbitai bot commented Apr 24, 2024 •

edited

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

hishamco commented Apr 24, 2024

Piedone commented Apr 24, 2024 •

edited

hishamco commented May 1, 2024

coderabbitai bot left a comment

Piedone May 2, 2024

hishamco May 2, 2024 •

edited

hishamco May 2, 2024

Piedone May 2, 2024

Piedone May 2, 2024

hishamco May 3, 2024

MikeAlhayek May 3, 2024

hishamco May 3, 2024

MikeAlhayek May 3, 2024

hishamco May 3, 2024 •

edited by MikeAlhayek

Avoid cloning the AutoPart pattern in the localized content #15810

Are you sure you want to change the base?

Avoid cloning the AutoPart pattern in the localized content #15810

Conversation

hishamco commented Apr 23, 2024 • edited by coderabbitai bot

Summary by CodeRabbit

hishamco commented Apr 23, 2024

MichaelPetrinolis left a comment • edited

Choose a reason for hiding this comment

hishamco commented Apr 23, 2024

hishamco commented Apr 24, 2024

coderabbitai bot commented Apr 24, 2024 • edited

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

hishamco commented Apr 24, 2024

Piedone commented Apr 24, 2024 • edited

hishamco commented May 1, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hishamco May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hishamco May 3, 2024 • edited by MikeAlhayek

Choose a reason for hiding this comment

hishamco commented Apr 23, 2024 •

edited by coderabbitai bot

MichaelPetrinolis left a comment •

edited

coderabbitai bot commented Apr 24, 2024 •

edited

CodeRabbit Configration File (`.coderabbit.yaml`)

Piedone commented Apr 24, 2024 •

edited

hishamco May 2, 2024 •

edited

hishamco May 3, 2024 •

edited by MikeAlhayek