-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Post-Installation Messages extension drop-down #4180
Conversation
…e correct extension ID, as set in the session
…on to view messages for
I tested the irst three patches, and for 3rd party components this seems to work as expected. I found only one small issue, if com_postinstall is opened for 3rd party component, clicking on Components menu link will still show messages for this component, however the title of the page changes to "Post-installation Messages for Joomla!". Anyway, from my view, expected behavior would be, if clicking tho components menu link, reset eid to Joomla core. |
I think what he meant is that the title changes fine to show "Post-installation Messages for com_foo" when changing using the select. But when you come back using the menu item, the title says "Post-installation Messages for Joomla!", but it still has the old filter applied. Imho it's fine that the filter is still applied. That's what I personally would expect. But the title should of course match the selection. The PR works fine from what I tested. I didn't test the new model method yet, however from glancing over the code it looks fine. If it's possible, it would be great to have the extension name translated in the title and dropdown. Seeing "com_foo" as the title and selection isn't that nice. Also I noted that apparently Seblod does enter a postinstall message upon installation ("welcome") and uninstallation ("has been uninstalled by admin xy"). It means that the dropdown now shows only a number (because the actual extension isn't there anymore) and we have no way of deleting those no longer needed messages. We can still hide them, but the number in the dropdown will always be there. There may of course be other extensions as well which do not remove their messages upon uninstallation. |
Also, you have a few codestyle issues. See the Travis results for the details. |
Let it be noted that I disagree with this code style. (int)$x is easier to understand than (int) $x in a long statement.
@Bakual Fixed the title and code style. Regarding the translation of the component name, of course it is translated. The prerequisite is that Joomla! has already loaded the extension's language file. This is done automatically for all components with a Components menu entry. I don't see how you could possibly get an untranslated com_foo? Steps to replicate, please? Regarding the messages of uninstalled extensions, the Seblod guys are doing something they shouldn't be doing. It should be self-understood that when you uninstall an extension its eid is no longer valid and its language files (pointed to by the post-installation messages) are no longer present. Do you want me to filter out uninstalled extensions? |
Ah I see. It's probably my fault then because I was lazy and just changed an existing message to a different
I agree that extensions should remove their messages on uninstall. |
…t-installation message keys
I tested the new model method by changing Akeeba Backup to use post-installation messages and found two small bugs which I fixed. I think that this PR has to be included in Joomla! 3.4, otherwise post-installation messages for third party components will not be very usable. |
Still one codestyle issue. |
Finally had some time to test this and everything works. A few things I wanted to point out:
|
That's actually not Bootstrap style. It's the
I think that would be a good improvement for another PR. I'm setting this to RTC since the new functionality is fine. Everything else can be improved later as well. |
Of course, I stand corrected. |
…nstallationMessage method. Closes #4180.
Merged into |
Please review the discussion in issue #4141 and reply here.
This PR implements an extension selection drop-down field in the post-installation messages view.
Moreover, this PR fixes the issues with the wrong extension ID being used when you are resetting the post-installation messages.
Finally, it adds a model method to add post-installation messages in the database. The upside of using it instead of directly inserting entries in the database is that it gives you intelligible error messages if you screw up :)
Close gh-4141 (hopefully this will close the other issue once we merge this PR)
@Bakual @n3t Please review this PR since you were involved in the original issue. Thanks!