-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Added billboard test link to admin page #20363
base: main
Are you sure you want to change the base?
Added billboard test link to admin page #20363
Conversation
Added billboard test link to "edit billboard" page. If "placement area" is selected as the post area then the link is displayed.
This comment was marked as outdated.
This comment was marked as outdated.
Thank you for opening this PR! We appreciate you! For all pull requests coming from third-party forks we will need to A Forem Team member will review this contribution and get back to |
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
I have read the CLA Document and I hereby sign the CLA |
As test post I used the last created article. Could you sugest the right way to create the normall test article? I imagine that this is some kind of pre-created article with a test title, test content, etc but I don't know how to implement this. Also I didn't understand how to test the fact that the right test billboard rendered since |
@@ -17,6 +17,7 @@ def new | |||
|
|||
def edit | |||
@billboard = Billboard.find(params[:id]) | |||
@test_article = Article.last |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only include published articles, so Article.published.last
should work.
@xao0isb thanks for this. I left one comment. Take a look at the CI to see if tests need to be adjusted. |
updateTestLinkVisibility(); | ||
|
||
const newlySelecetedPlacementAreaValue = placementAreaSelect.value; | ||
testPlacementAreaLink.href = testPlacementAreaLink.href.replaceAll(previouslySelecetedPlacementAreaValue, newlySelecetedPlacementAreaValue); |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML
Uffizzi Ephemeral Environment
|
762cbfb
to
ad884b5
Compare
@benhalpern I have finalised the code and fixed the tests. The CLA bot still fails on commit where I didnt configure my mail and used auto generated. How can I fix this? Maybe I should make a new commit with the name that was used in the commit with auto generated mail? |
@xao0isb Did you sign the CLA? |
What type of PR is this? (check all applicable)
Description
Added billboard test link to "edit billboard" page. If "placement area" is selected as the post area then the link is displayed.
Related Tickets & Documents
UI accessibility checklist
If your PR includes UI changes, please utilize this checklist:
Critical
andSerious
issues?For more info, check out the
Forem Accessibility Docs.
Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above.
request.body
returns the html code without the billboard code itself because it does not have time to load