Solving conflicts from merge with develop. Test not passing. #1899
Conversation
|
Hi @mit0110, thanks! I looked at third_party.js at the line number indicated, and it seems like there was an issue with $document not being defined. The stack trace also refers to explorationContextServiceSpec.js, so I looked at that too. It seems like you are stubbing out $document in that file. This basically messes things up for other third-party libraries that want to make use of $document. Is it possible to stub it out less invasively? |
|
Thanks! It was the stubbing. Now I fixed the frontend and backend tests, and I'm working on the integration tests. The UI is still broken because it has two footers. Also, we need new translations for the new content. I think it's a good point if you want to make a review, the rest of the changes should be more straightforward. Thanks! |
|
Sounds good! Just one question: should I merge this into i18n-issue-103 post-review, if everything is good? |
| self.assertEqual(response.status_int, 200) | ||
| csrf_token = self.get_csrf_token_from_response(response) | ||
| print "lalalala", csrf_token |
|
Hi @mit0110, thanks, this looks great! I don't have many comments. One general question: there's a bunch of I18N_* strings that relate to the GALLERY. Can we rename these to use LIBRARY instead? Thanks! |
|
Hi @seanlip! Now all tests are working, but the UI is a problem. In particular, now there are two footers and I'm not sure how to combine them. Specially because the new footer only appears in certain pages. Do you think someone designer can take a look at this? I really like the https://www.airbnb.com/ footer, but still I'm not sure how to display it without the other footer, like in the library page for example. I think that at this point, the branch can be merged into i18n-issue-103 and fix the UI and add the missing translations in different PRs. Let me know if you have further comments, thanks! |
|
LGTM. Thanks! Merging. |
Hi @seanlip! I solved the conflicts but there is one frontend test that I can't fix. This is not a part of the code that was affected by the merge. Only one backend test is failing, and I haven't tried the integration tests jet but they are broken for sure with the gallery -> library renaming.
I made a commit with the partial fixes. Also, I'm sending attached the output of the test that is failing. Do you have any idea of what may be the problem? I can ask in the list tomorrow.
Thanks!
test_output.txt