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

Fix display of IP addresses in server module; fix binary IP handling in MasterForm #387

Merged
merged 11 commits into from Feb 21, 2019

Conversation

Projects
None yet
3 participants
@aj-gh
Copy link
Contributor

aj-gh commented Jan 24, 2019

Looks like this was broken since the database field was switched to varchar(16) for future IPv6 support in commit 16264ba

Fix display of IP address in server module
Looks like this was broken since the database field was switched to varchar(16) for future IPv6 support.
@M4LuZ

This comment has been minimized.

Copy link
Collaborator

M4LuZ commented Jan 24, 2019

Seems that the code repo for PHP_CodeSniffer is unreachable atm. But code looks fine (to be tested)

@andygrunwald andygrunwald added the bug label Jan 26, 2019

aj-gh and others added some commits Jan 26, 2019

@@ -13,7 +13,7 @@
$ms2->AddSelect('u.userid');
$ms2->AddResultField(t('Name'), 's.caption');
$ms2->AddResultField(t('Servertyp'), 's.type', 'ServerType');
$ms2->AddResultField(t('IP-Adresse / Domain'), 's.ip');
$ms2->AddResultField(t('IP-Adresse / Domain'), 'INET6_NTOA(s.ip) AS ip');

This comment has been minimized.

@andygrunwald

andygrunwald Jan 27, 2019

Collaborator

Shouldn't be this realip then? Or doesn't it have a direct relation to the other usages?

This comment has been minimized.

@aj-gh

aj-gh Jan 27, 2019

Author Contributor

Well... I initially fixed this code (overview page) and then noticed that the details view was still broken.
I didn't want to go far from "ip" here because that's how that field was always called.
However, in show_details.php a select * is done which also returns "a.ip" as "ip". Now if I would have called the INET6_NTOA result "ip" as well that query would return both columns as "ip" ... which would become rather interesting I guess; at least confusing. I then looked at the amount of columns of the server table and ended up with calling that field realip.
Reconsidering this, I guess the best way would be me changing this in show_details.php from a SELECT * to specifying all the columns and also calling the resulting field "ip" there.
What do you think?

This comment has been minimized.

@andygrunwald

andygrunwald Jan 30, 2019

Collaborator

Reconsidering this, I guess the best way would be me changing this in show_details.php from a SELECT * to specifying all the columns and also calling the resulting field "ip" there.

Yes. I am for this. Personally, I like to avoid SELECT * and to make it explicit which fields you are requesting. Often this makes things easier. Do you want to adjust it in this PR or in a new one?

This comment has been minimized.

@aj-gh

aj-gh Jan 30, 2019

Author Contributor

Will do it here.

aj-gh added some commits Jan 30, 2019

@aj-gh aj-gh changed the title Fix display of IP addresses in server module [WIP] Fix display of IP addresses in server module Jan 30, 2019

@aj-gh

This comment has been minimized.

Copy link
Contributor Author

aj-gh commented Jan 30, 2019

Not done yet; "Edit" is still broken.

@M4LuZ M4LuZ added the WIP label Jan 31, 2019

@aj-gh

This comment has been minimized.

Copy link
Contributor Author

aj-gh commented Jan 31, 2019

Not done yet; "Edit" is still broken.

.. and fixed.

@aj-gh aj-gh changed the title [WIP] Fix display of IP addresses in server module Fix display of IP addresses in server module; fix binary IP handling in MasterForm Jan 31, 2019

andygrunwald added some commits Jan 31, 2019

Show resolved Hide resolved inc/Classes/MasterForm.php

@M4LuZ M4LuZ added this to the LanSuite 5.0 RC milestone Feb 5, 2019

andygrunwald added some commits Feb 5, 2019

@M4LuZ M4LuZ merged commit fbf4fdd into lansuite:master Feb 21, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aj-gh aj-gh deleted the aj-gh:server-fix-ip-address-display branch Feb 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.