Skip to content
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

Ambiguity in LocalizedTaxonomyController action names #8785

Merged
merged 10 commits into from
May 16, 2024

Conversation

AndreaPiovanelli
Copy link
Contributor

This fixes an ambiguity issue for the GetTaxonomy action of LocalizedTaxonomyController. There was a "new GetTaxonomy" action in AdminLocalizedTaxonomyController too that was ambiguous, failing to properly work. For this reason, the action has been removed from AdminLocalizedTaxonomyController and the admin filter is added via override of the ApplyPreRequest function.

@BenedekFarkas
Copy link
Member

Can you provide repro steps to compare before/after?

@AndreaPiovanelli
Copy link
Contributor Author

Can you provide repro steps to compare before/after?

The issue was easily reproducible when opening the edit page of an item containing a TaxonomyField. The ajax call in TaxonomyFieldList shape (https://github.com/LaserSrl/Orchard/blob/8ed7b67f1fc865e60b1de658b085fb49efb45151/src/Orchard.Web/Modules/Orchard.Taxonomies/Views/EditorTemplates/Fields/TaxonomyFieldList.cshtml#L13) failed, showing the "Loading taxonomy failed" alert message at line 136 of the same file. The error wasn't logged by Orchard, because it's a MVC error, but you can see it in the browser console:
Loading taxonomy fail message:
image
The error displayed was the following:
The current request for action 'GetTaxonomy' on controller type 'AdminLocalizedTaxonomyController' is ambiguous between the following action methods:

The current request for action 'GetTaxonomy' on controller type 'AdminLocalizedTaxonomyController' is ambiguous between the following action methods:
System.Web.Mvc.ActionResult GetTaxonomy(System.String, System.String, Int32, System.String, System.String) on type Orchard.Taxonomies.Controllers.AdminLocalizedTaxonomyController
System.Web.Mvc.ActionResult GetTaxonomy(System.String, System.String, Int32, System.String, System.String) on type Orchard.Taxonomies.Controllers.LocalizedTaxonomyController

I didn't notice this problem before PR #8780 was merged, because of the javascript syntax error blocking the proper execution of the code.

Copy link
Member

@BenedekFarkas BenedekFarkas left a comment

Choose a reason for hiding this comment

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

Two small things noted, otherwise works great!

@BenedekFarkas
Copy link
Member

@AndreaPiovanelli please check the suggested changes and/or enable edits from maintainers.

@AndreaPiovanelli
Copy link
Contributor Author

@BenedekFarkas I took some time to test your suggestions in our environment and have now applied the changes you requested to current pr branch.
I had to apply the last one separatedly (not via the "commit suggestion" button) because git disabled that button as apparently it was an "outdated suggestion" for it.

@BenedekFarkas
Copy link
Member

OK, thanks!

@BenedekFarkas BenedekFarkas merged commit fc8b681 into OrchardCMS:dev May 16, 2024
3 checks passed
@AndreaPiovanelli AndreaPiovanelli deleted the fix/taxonomyactionambiguity branch June 7, 2024 10:43
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.

2 participants