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

Resource create #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Resource create #27

wants to merge 2 commits into from

Conversation

kdimopulu
Copy link
Collaborator

No description provided.

@kdimopulu kdimopulu marked this pull request as ready for review November 22, 2024 10:57
@blacksmith-welder blacksmith-welder changed the title Resource Create Resource create Nov 22, 2024
@@ -65,6 +65,24 @@
end
end

When 'the user fills in {string} with {string} and selects {string} in the {string} form' do |label, value, selected_value, form_title|
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bit of a nitpick but the user fills in the {string} field would be more clear to me here

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have intentionally left the word field out since it's redundant. Everything that can be filled, selected, and generally is part of a form, can be considered a field.
The same applies to the user clicks on {string}, which is intentionally ambiguous, as it may be a button or a link, or it may look like a button but may contain a link, or the opposite.
The same applies to the word record for entities of the system. We use capitalized words for all the records like: Accession, Resource, Extent, etc.

We want the Gherkin to be as clear, concise, and compact as possible, and not so much the step definitions, which if you look at the block arguments, is quite clear what they refer to.

If we suffix the word field, shouldn't we do the same for all the {string}s, and for all form user input definitions (selects etc.)?
In my opinion, this redundancy reduces clarity:
'the user fills in {string} field with {string} value and selects {string} from the dropdown menu here? in the {string} form' do |label, value, selected_value, form_title|

If you think we must apply this change, I believe we should open a new PR, to apply these changes everywhere, to avoid inconsistency between various step definitions that use the current phrasing.

@@ -129,6 +147,10 @@
expect(page).to have_field(label, with: value)
end

Then 'the {string} has a unique value' do |label|
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly here; the {string} field has a unique value

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.

3 participants