ipatests: webui: test_host.py Fix expected item visibility

[miskopo]

Fix test to expect request_cert to be visible in host drop-down menu.

Resolves: https://pagure.io/freeipa/issue/8504

Signed-off-by: Michal Polovka mpolovka@redhat.com

[flo-renaud]

@miskopo
I don't understand the rationale for this change. In a CA-less environment, I would expect the menu item "Host/New certificate" to be hidden or grayed out but not visible. Furthermore there is no associated ticket...

[flo-renaud]

I finally got it... I think you should call assert_action_list_action(visible=True, enabled=False) as the menu item is visible but grayed out.
The tests are failing because the temp commit is using class: RunPytest instead of RunWebuiTests.
Note that we don't have any upstream test actually exerting this test (it's skipped because the RunWebuiTests are launched with an embedded self-signed CA).

[miskopo]

Thank you @flo for comments, I've incorporated your suggestions.
Currently running temp_commit for test_host.py

[miskopo]

Okay, I see it now, ca_less tests are not executed.

[pvoborni]

Are we sure that the test change is correct? I.e. is there a valid reason to change the test? E.g. was there a change in behavior of Web UI or something else to display the items differently? I'm bit out of touch, so I don't recall that. @serg-cymbaluk do you know?

If I look into Web UI source code. I see still an indication that if ca is_enabled API command returns false (should set ra_disabled state in Web UI facet by the evaluator), the action(s) should be hidden (hide_cond fulfilled). I wonder if there is a regression in treating result of this command as always true (e.g. if it is returned as a string or object).

IPA.cert.request_action = function(spec) {
    spec = spec || {};
    spec.name = spec.name || 'request_cert';
    spec.label = spec.label || '@i18n:objects.cert.new_certificate';
    spec.enable_cond = spec.enable_cond || ['ra_enabled'];
    spec.hide_cond = spec.hide_cond || ['ra_disabled'];

IPA.cert.revoke_action = function(spec) {

    spec = spec || {};
    spec.name = spec.name || 'revoke_cert';
    spec.label = spec.label || '@i18n:objects.cert.revoke_certificate_simple';
    spec.enable_cond = spec.enable_cond || ['has_certificate'];
    spec.disable_cond = spec.disable_cond || ['certificate_revoked'];
    spec.hide_cond = spec.hide_cond || ['ra_disabled'];
    spec.confirm_dialog = spec.confirm_dialog || IPA.cert.revoke_dialog;
    spec.needs_confirm = spec.needs_confirm !== undefined ? spec.needs_confirm : true;

IPA.cert.remove_hold_action = function(spec) {

    spec = spec || {};
    spec.name = spec.name || 'remove_hold_cert';
    spec.label = spec.label || '@i18n:objects.cert.remove_hold';
    spec.enable_cond = spec.enable_cond || ['has_certificate', 'certificate_hold'];
    spec.hide_cond = spec.hide_cond || ['ra_disabled'];

Later:

IPA.cert.certificate_evaluator = function(spec) {

    spec.name = spec.name || 'has_certificate_evaluator';
    spec.event = spec.event || 'certificate_loaded';

    var that = IPA.state_evaluator(spec);

    that.on_event = function(certificate) {

        var old_state, record, state, value, loaded_value;

        old_state = that.state;
        that.state = [];

        if (certificate && certificate.certificate) {
            that.state.push('has_certificate');

            if (certificate.revocation_reason !== undefined) {
                that.state.push('certificate_revoked');

                if (certificate.revocation_reason === 6) {
                    that.state.push('certificate_hold');
                }
            }
        }

        if (IPA.cert.is_enabled()) {
            that.state.push('ra_enabled');
        } else {
            that.state.push('ra_disabled');
        }

        that.notify_on_change(old_state);
    };

    return that;
};

IPA.cert.is_enabled = function() {
    return !!IPA.ca_enabled;
};

batch.add_command(rpc.command({
            entity: 'ca',
            method: 'is_enabled',
            on_success: function(data, text_status, xhr) {
                that.ca_enabled = data.result;
            }
        }));

[serg-cymbaluk]

@pvoborni @miskopo

The test changes are correct. Meanwhile there is an issue:
The condition doesn't work for hosts and services, but works for users.
If you go first to hosts and then navigate for users, the condition wouldn't work also for users.

[pvoborni]

The question was why is it a valid change? Nobody mentioned the reason yet.

[flo-renaud]

@pvoborni @miskopo @serg-cymbaluk

The behavior of the drop-down menu is different depending on the version. The "New certificate" action for users/services/hosts is sometimes displayed (=visible), sometimes grayed out (=visible but disabled), sometimes not displayed (=hidden):

Version	User "new certificate"	Host "new certificate"	Service "New certificate"
RHEL 7.9 (ipa-server-4.6.8-5.el7.x86_64)	visible	visible	visible
RHEL 8.3 (ipa-server-4.8.7-12.module+el8.3.0+8222+c1bff54a.x86_64).	visible	visible	visible
Fedora 33 (freeipa-server-4.8.10-6.fc33.x86_64)	disabled	hidden	hidden

This is the result of issues https://pagure.io/freeipa/issue/8369 and https://pagure.io/freeipa/issue/8203. They are now fixed on ipa-4-6, ipa-4-8, ipa-4-9 and master branches (the right behavior is the one seen on 4.8.10), but not yet delivered to RHEL. As a summary, I think we don't need this fix anymore as the test is already consistent with the code in the branches.

[abbra]

I am closing this PR based on the investigation by @flo-renaud in #5110 (comment)