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

Fix(Footer): Less HTML #518

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

Fix(Footer): Less HTML #518

wants to merge 13 commits into from

Conversation

chrisma
Copy link
Contributor

@chrisma chrisma commented Mar 26, 2019

No description provided.

@chrisma chrisma added the WIP work is still in progess for this PR label Mar 26, 2019
@chrisma
Copy link
Contributor Author

chrisma commented Mar 26, 2019

Classic example of a brittle test case here: spec/features/requests/accept_reject_request_spec.rb:49 fails, as it no longer finds the string Info, which was only ever contained in the footer's "More Information" heading...

@chrisma chrisma added the bug Something isn't working label Mar 26, 2019
@bdaase
Copy link
Contributor

bdaase commented Apr 1, 2019

Classic example of a brittle test case here: spec/features/requests/accept_reject_request_spec.rb:49 fails, as it no longer finds the string Info, which was only ever contained in the footer's "More Information" heading...

So we should just remove this test and add another one which checks that the links are available?

@chrisma
Copy link
Contributor Author

chrisma commented Apr 2, 2019

So we should just remove this test and add another one which checks that the links are available?

Well, the test accept_reject_request_spec.rb only ever passed because of a string in the footer, that is unrelated to what is actually being tested. However, accepting and rejecting requests is the core workflow of this application and should be thoroughly tested. The test should be fixed so that it actually tests the assumed behavior.

@bdaase
Copy link
Contributor

bdaase commented Apr 3, 2019

So we should just remove this test and add another one which checks that the links are available?

Well, the test accept_reject_request_spec.rb only ever passed because of a string in the footer, that is unrelated to what is actually being tested. However, accepting and rejecting requests is the core workflow of this application and should be thoroughly tested. The test should be fixed so that it actually tests the assumed behavior.

Oh, now I see what you mean 👍

@bdaase
Copy link
Contributor

bdaase commented Apr 3, 2019

@chrisma Should be fixed now.

end

it 'shows rejection information' do
expect(page).to have_text('Info')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should (re)add a test that the rejection information is actually displayed to the user

@bdaase
Copy link
Contributor

bdaase commented Apr 12, 2019

@chrisma I added a test for that, which is currently failing. The problem is, that clicking the "Reject" button currently also results in an acceptance of the request (on hart-dev).
Unluckily, I can not debug this problem on my own machine, because I always get the following error when either creating, accepting or rejecting a request:
photo5296515629358098994
@BraunTom Do you have an idea what might have happend here?

@bdaase bdaase added the blocked label Apr 12, 2019
@bdaase
Copy link
Contributor

bdaase commented Apr 12, 2019

@chrisma I added a test for that, which is currently failing. The problem is, that clicking the "Reject" button currently also results in an acceptance of the request (on hart-dev).
Unluckily, I can not debug this problem on my own machine, because I always get the following error when either creating, accepting or rejecting a request:
photo5296515629358098994
@BraunTom Do you have an idea what might have happend here?

Due to @BraunTom related to the form in form on the requests page (which is not valid https://stackoverflow.com/questions/555928/is-it-valid-to-have-a-html-form-inside-another-html-form).

@chrisma
Copy link
Contributor Author

chrisma commented Apr 29, 2019

One large form with two submit buttons, one with the value "accept", one with the value "reject".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked bug Something isn't working WIP work is still in progess for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants