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

Remove filter column and add the extra explanation text #642

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

MKodde
Copy link
Contributor

@MKodde MKodde commented Jun 18, 2024

  1. The filter column is removed from the template as it is out of scope for now.
  2. An additional translatable explanation text was added
  3. Webtests have been reinstated

@MKodde MKodde force-pushed the feature/surfconext-verantwoordelijke-remove-filter branch 2 times, most recently from 89bee6d to a5f9409 Compare June 18, 2024 13:13
Copy link
Contributor

@parijke parijke left a comment

Choose a reason for hiding this comment

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

Maybe consider renaming the weird method name

@@ -49,6 +49,12 @@ public function shouldFailCreateIssue(): void
$this->failIssueCreation = true;
$this->storeData();
}
public function shouldNotFailCreateIssue(): void
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a weird method name. No method should ever fail, but they do... not clear to me

Copy link
Contributor Author

@MKodde MKodde Jun 18, 2024

Choose a reason for hiding this comment

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

Haha it is weird indeed, this a method that modifies the behavior of a fixture. You can enable it to simulate Jira being down. I think it precisely describes its (test) purpose. And I'd like to keep it as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not call it enableJiraDownSimulation() then?

Copy link
Contributor Author

@MKodde MKodde Jun 20, 2024

Choose a reason for hiding this comment

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

I find this more fitting in functional tests, but my pardigm is mainly gherkin oriented. I feel this fits the bill good'nuf

@MKodde MKodde force-pushed the feature/surfconext-verantwoordelijke-authz branch 2 times, most recently from 8c12aae to e66a0ff Compare June 19, 2024 06:47
Base automatically changed from feature/surfconext-verantwoordelijke-authz to main June 19, 2024 06:48
It will not be included on the MVP of the surfconext representative
page. No need to include it, leaving extra space for the SP/IdP
entities.
And adjust other webtests to the new Fixtures created for the surfconext
representative page
@MKodde MKodde force-pushed the feature/surfconext-verantwoordelijke-remove-filter branch from a5f9409 to 928e35b Compare June 19, 2024 06:51
Copy link
Contributor

@parijke parijke left a comment

Choose a reason for hiding this comment

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

Why not call it enableJiraDownSimulation() then?

@MKodde MKodde merged commit 747785e into main Jun 20, 2024
2 checks passed
@MKodde MKodde deleted the feature/surfconext-verantwoordelijke-remove-filter branch June 20, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants