search

virtualmin / virtualmin-gpl

Virtualmin web hosting control panel for Webmin
https://www.virtualmin.com
GNU General Public License v3.0
331 stars 102 forks source link

Fix warnings and messages #953

Closed iliajie closed 2 weeks ago

iliajie commented 2 weeks ago

Hello Jamie,

This is a related PR that we should discuss here: https://github.com/virtualmin/virtualmin-pro/pull/47.

jcameron commented 2 weeks ago

Let me know when this is ready for re-review based on our VC discussion today ..

iliajie commented 2 weeks ago

Let me know when this is ready for re-review based on our VC discussion today ..

Yes, it’s ready now! Thanks for your suggestion—it works perfectly! I also looked into the user making_changes and realized there’s a much simpler fix.

Please take a look. The main changes to review are:

Domain related: https://github.com/virtualmin/virtualmin-gpl/pull/953/commits/363cc05fda1ce5c2d55d25f773ecbabdddadb96c https://github.com/virtualmin/virtualmin-gpl/pull/953/commits/dd4cdeacda18d8c652e5905e45ebc54cdd6fd9a6

User related: https://github.com/virtualmin/virtualmin-gpl/pull/953/commits/b7de817a5092d0965e4bf138f830a17c2965b2d2

iliajie commented 2 weeks ago

Jamie, let's also discuss this https://github.com/virtualmin/virtualmin-gpl/pull/953/commits/a802def57571a12a447f83e543e63b79dba7fe7d change as potentially acceptable.

I find it simpler and more straightforward because we just exit cleanly without potentially running anything. It also requires no wrapper for useradmin, as we discussed earlier.

iliajie commented 2 weeks ago

I just realized that we already test modifier pages here.