-
Notifications
You must be signed in to change notification settings - Fork 500
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
Support Form: math challenge doesn't work from some pages #3036
Comments
@scolapasta thanks for agreeing during this morning's standup to take a look at this issue. Again, the easiest fix is to remove the @scolapasta during the meeting I said I'd also give two more issues to you that were kicked back to me. Here are links to the comments from @kcondon:
Since the 2939-shib branch that pull request #3025 is based on is particularly effected, I would suggest fixing the problem there. |
Giving this to @mheppler. In 4.3 for the notification, and in the shib branch for all the other support references, he added these links to Support. (403 and 404 used to say, click support link on the header). However these references in the bundle only open up the dialog box and don't do what happens when you click on the link in the header, i.e call to the server to initialize the popup. So (and @pdurbin mentions some of this above we need to: The notification is a different case. In particular, we do also send out notification texts in e-mail. So we may not want to have this in the first place, as you're not in the app, i.e. can't open up a link. |
Here is the code from the header:
In particular, some of the properties would be set differently depending on where you clicked the page from, so fir example, we could prepopulate messageSubject to be "404" if from that page. This, of course, would suggest that c above would be the course of action. |
After reviewing all the comments and looking at the code, I believe that best of the three options outlined above is "c) put the logic on each of the links on the 403, 404, shib login, and account pages to initialize". Passing this ticket to Gustavo to implement this or better yet find an even more dynamic solution that would allow bundle messages with contact support links to be easily adding going forward, without needing a custom solution. |
+1 to @mheppler's comment. I do not think it should be changed to "click the link in the header". Would not be a good user experience practice. |
When @scolapasta gave this issue to me the other day we discussed it and decided to
In total there are 5 places to check:
I used my best judgement to fix the notification regression in 4.3 that was introduced in #2966 (that issue should be tested or retested). Here's a before and after: Notification BeforeNotification AfterPassing to QA. |
Discussed with @pdurbin and to avoid dead ends with the Support text, he will change the text to say, "or email (insert system email address link) for assistance." |
I'm happy with the code in the 3036-support-form-math-bug so I went ahead and created pull request #3573 based on it. I did the following:
Note that I could not test the math bug scenario mentioned in #3265 because I got a proper error message when I tried to use more than 30 characters for a client nickname. In short, I believe the math challenge now works everywhere. If there's a place where it doesn't work, I don't know where it is. Passing to QA. |
Tested:
This appears to be working. Closing. |
@pdurbin Works but needs to have merge conflicts resolved. |
All set. Closing. |
This was pulled back into the development column because the fix broke publish. This has been fixed, but a different approach will need to be taken here. |
I'm moving this into the backlog. We'll address this in a future sprint. |
This issue goes on and on. Let's create a fresh one when we're ready to estimate it. Closing. |
Closing in favor of #6307 |
In #1327 we added a simple math challenge to slow spammers down from filling out the support form but in some places the operands are now missing. For example on https://demo.dataverse.org running v. 4.3 build 22-1e451ff if you sign up, click the "welcome" notification, and click "Dataverse Support", you don't see any number to add together:
As noted by @kcondon this regression appears in additional places in the Shib-related pull request at #3025 as of 4d332e0:
Interestingly, if you click the normal "Support" link in the header first, the problem goes away. From a quick look it seems like the problem started happening when we staring using constructions like
<a href="#" onclick="event.preventDefault();PF('contactForm').show();">Dataverse Support</a>
in Bundle.properties. I'm not actually sure what the fix should be. Here's a list of where this construction is used in pull request #3025:The text was updated successfully, but these errors were encountered: