Skip to content

Refactor alias and title increment logic in StringHelper #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 3.x-dev
Choose a base branch
from

Conversation

charvimehradu
Copy link

@charvimehradu charvimehradu commented Mar 3, 2025

Pull Request for Issue #45011

Summary of Changes

Modified the increment function in the StringHelper file with the same logic @chmst suggested. First, the function checks the format of the string by determining the style. If the default style is used, it checks if the string contains a number inside parentheses. If a number is present, it increments the number inside the parentheses. If no number is found, it appends (2) to the title. If the dash style is used, it checks if the string ends with a number preceded by a hyphen. If a number is present, it increments it. If no number is found, it appends -2 to the alias. I would appreciate any help or suggestions to ensure this solution works as expected across various use cases.

Testing Instructions

When merged with joomla-cms,
Example Test: Create an article that has
Title: August 2024
Alias: august-2024

Save as copy
Title: August 2024 (2)
Expected Alias: august-2024-2

This method supports two formats for string incrementing:

  1. Title Format (Default Style): For titles, the increment will be handled inside parentheses.

    • Example: "dog" becomes "dog (2)", and "dog (2)" becomes "dog (3)".
    • If the title does not already have a number inside parentheses, it appends (2) by default.
  2. Alias Format (Dash Style): For aliases, the increment will be handled with a hyphen (-).

    • Example: "dog-2" becomes "dog-3".
    • If the alias does not have a number after the hyphen, it appends -2 by default.
alias_issue.2.mp4

Documentation Changes Required

@Hackwar
Copy link
Contributor

Hackwar commented Mar 3, 2025

Please have a look at the unit tests in Drone. This change makes them fail. That means your change wouldn't be backwards compatible and thus can't be accepted into 3.x-dev.


if (preg_match($rxSearch, $string, $matches)) {
$n = empty($n) ? ($matches[1] + 1) : $n;
$string = preg_replace($rxReplace, sprintf($rxReplace, $n), $string);

Choose a reason for hiding this comment

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

You'll get an error here, you are replacing the n (value number) in the replacement string "-%d" which is not a valid replacement string for the preg_replace function, because it doesn't have the required delimiter -.

Your code will work, and replace report-2025 with report-2025-2, how it works?
Because, when it gets the error I mentioned, it will result in an empty string
Then when the empty alias reaches Content.php it will enter here
if (trim($this->alias) == '') { $this->alias = $this->title; }
Now the alias is typical as the title, lets say the title is report 2025 (2)

This following method will make behave the alias to be a safe URL, i.e. it will remove spaces and brackets and replace them with hyphen so report 2025 (2) will be report-2025-2
$this->alias = ApplicationHelper::stringURLSafe($this->alias, $this->language);

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing that out! Looking for some better solution. Will revert back soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants