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

Automatically set lightspeed and inline suggestion checkboxes to enabled when user clicks Connect #1789

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mabashian
Copy link
Member

@mabashian mabashian commented Jan 27, 2025

https://issues.redhat.com/browse/AAP-31064

The basic gist of this change is that when we catch the "connect" message and before we initiate the login flow, we update ansible.lightspeed.enabled and ansible.lightspeed.suggestions.enabled to be true (if they're not already true).

I found that I needed to update the status bar logic since that gets triggered by the programatic settings update and the scenario where a user is not yet logged in wasn't explicitly handled. Without this change, the modal never appears after clicking Connect

As far as the tests go, I extended what was already there in the ui tests to include two scenarios:

  1. Clicking Connect when both checkboxes are already checked
  2. Clicking Connect when both checkboxes are unchecked

In both cases the tests confirm that the checkboxes are checked after login

@mabashian mabashian force-pushed the mabashian/AAP-31064 branch from 6ae3566 to 5eba635 Compare January 27, 2025 21:02
@mabashian mabashian force-pushed the mabashian/AAP-31064 branch from 9f321c7 to 078808e Compare January 29, 2025 13:51
@mabashian mabashian force-pushed the mabashian/AAP-31064 branch from 078808e to ca38c75 Compare January 29, 2025 13:51
);
}

if (!lightSpeedSuggestionsEnabled) {
Copy link
Contributor

@goneri goneri Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, if the value is already set to true, nothing bad will happen.

I don't think we need to read lightSpeedSuggestionsEnabled first. This increases the complexity of the code, and reduce the test coverage, without bringing any tangible value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants