- Issue created by @dunx
- First commit to issue fork.
- 🇧🇪Belgium wouters_f Leuven
Thanks for your feedback!
I'll see if I can make the initial process a bit more seamless. - 🇧🇪Belgium wouters_f Leuven
I've provided some more failsafes.
If no default chat provider is configured, I'll refrain from showing the links and (and the error).It only shows the error message that you should configure the chat model.
Would you be able to test this and verify that this is better?Or you have a proposal to do it differently?
- 🇬🇧United Kingdom seogow
The changes make the code more resilient to misconfiguration by:
- Properly handling the case when no model is configured.
- Properly handling the case when the default model string is empty.
- Only attempting to create the translation link when we have a valid model name.
This aligns with the issue's "Proposed resolution" of making the module more resilient to misconfiguration.
- 🇬🇧United Kingdom dunx
Steps to set up for test
- Installed D11 Umami
- Installed and enabled key module at current
- (Installed and enabled admin_tools)
ddev composer require 'drupal/ai:1.0.x-dev'
ddev composer config repositories.ai vcs https://git.drupalcode.org/issue/ai-3498757.git
ddev composer require drupal/ai:349875-aitranslate-resilience-dev
ddev drush en ai ai_translate -y
- No configuration of the ai module or sub-module has been performed.
Steps to test
I created an article and then clicked on the Translate tab.
An error is no longer displayed :)
Instead I see this helpful error message, which is an improvement.
I clicked on that link (/en/admin/config/ai/settings).
There's nothing in any of the Dropdowns as expected as I haven't set Open AI up yet (or any other provider).
So I click on the "configure" link here.
And I get the same "Access denied" error as before.
What I need to do is download and enable the ai_provider_openai module. But I should not see an access denied error, just because I haven't.
Summary
The first issue has been fixed, but the second issue remains unresolved.
- 🇧🇪Belgium wouters_f Leuven
Hi Dunx,
glad this fixed your first "issue".Concerning your second issue:
What you're describing has nothing to do anymore with the ai_translate module.
You can't configure providers that you haven't installed so the 404 makes sense in some way.I'm considering i'm done with this ticket, i cant make the ai_translate submodule more config resilient than what i've proposed.
I propose the following steps:
1 Allow Marcus to merge this and reduce the scope to your "issue 1". so the fix can get into the codebase.
(STEPS: put into review and tested again)2 make a separate issue for your second issue, not related to the ai_translate submodule.
Don;t get me wrong, i agree with that we should make it as easy as possible, but combining issues will just make everything slower.
- 🇬🇧United Kingdom dunx
It's not a 404, it's an access denied. But I hear you. Happy to make some improvement with the first issue and much appreciated of course.
I can raise the second issue separately.Will update this to RTBC.
- 🇬🇧United Kingdom dunx
@wouters_f you said "What you're describing has nothing to do anymore with the ai_translate module. You can't configure providers that you haven't installed so the 404 makes sense in some way."
Where should I raise this access denied issue then if it's not against this module?
- 🇧🇪Belgium wouters_f Leuven
This is a ai module thing so it's the same issue queue, but different component (Ai core)
- 🇩🇪Germany marcus_johansson
Hi @dunx - there is already a second issue being worked on here ✨ Add useful message on providers page when no provider enabled Active , for the #2, so no need for a new issue for that.
- 🇩🇪Germany marcus_johansson
Since seogow has done code review and dunx has confirmed it solves the issue with AI Translate this is getting merged.
Automatically closed - issue fixed for 2 weeks with no activity.