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.

Sign up for GitHub

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

Jump to bottom

Fix #177 CPCSS notice has scenario that won't work correctly now due to the login structure #180

Merged
merged 3 commits into from
Jan 8, 2025
Merged

Fix #177 CPCSS notice has scenario that won't work correctly now due to the login structure #180

merged 3 commits into from
Jan 8, 2025

Conversation

Khadreal
Copy link
Contributor

Description

Add another username/password for second admin user.

Fixes #177

Type of change

  • Bug fix (non-breaking change which fixes an issue).

Detailed scenario

Check #177

Technical description

Documentation

Add option for admin 2 user for cpcss test.

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

@Khadreal Khadreal requested a review from a team December 31, 2024 10:34
@Khadreal Khadreal self-assigned this Dec 31, 2024
@Khadreal Khadreal added the bug Something isn't working label Dec 31, 2024
wordpressfan
wordpressfan previously approved these changes Jan 3, 2025
View reviewed changes
Copy link
Contributor

@wordpressfan wordpressfan left a comment

Choose a reason for hiding this comment

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

Approved with two minor change requests

utils/page-utils.ts Outdated Show resolved Hide resolved
utils/page-utils.ts Outdated Show resolved Hide resolved
@Mai-Saad
Copy link
Contributor

Mai-Saad commented Jan 3, 2025

@Khadreal Thanks for the PR. Smoke test and CPCSS are good there (failed test in smoke is due to error in debug log that shall be ignored on other ticket). if we have some changes due to CR, please make sure to share results of smoke and cpcss after changes
Screenshot from 2025-01-03 15-39-33
Screenshot from 2025-01-03 15-19-22

@Mai-Saad
Copy link
Contributor

Mai-Saad commented Jan 8, 2025

CPCSS is passing now , smoke has failed expected (either export or ignored error)
Screenshot from 2025-01-08 11-56-58
Screenshot from 2025-01-08 11-38-25

@Mai-Saad Mai-Saad merged commit 36f8526 into develop Jan 8, 2025
2 checks passed
@Mai-Saad Mai-Saad deleted the fix/177-cpcss-notice branch January 8, 2025 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeawhanlee jeawhanlee jeawhanlee approved these changes

@wordpressfan wordpressfan Awaiting requested review from wordpressfan

Assignees

@Khadreal Khadreal

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CPCSS notice has scenario that won't work correctly now due to the login structure
4 participants
@Khadreal @Mai-Saad @jeawhanlee @wordpressfan