-
Notifications
You must be signed in to change notification settings - Fork 103
Sprint2 web 2nd nov #474
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
Sprint2 web 2nd nov #474
Conversation
Previously, 'locked' images (E.g. other's images in read-only group) did not launch the image viewer when you click on the icon in the tree. This is fixed.
|
@will-moore, tracking backwards, I've finally cross-checked where the various commits, but if possible, it'd be nice if we could avoid these situations (i.e. modified rebasings, cherry-pickings, etc). Might be worth talking through what happened to come up with a way to prevent it next time. Note: I haven't checked this PR just where the commits are. |
|
Everything looks good except for the first test which I haven't managed to do - rendering exception. Is there in easy way to force one? Otherwise this was tested in PR464. |
|
@will-moore, worth trying to reproduce? |
|
@ximenesuk I was simulating an exception by adding a raise to the code! Not sure how @chris-allan tested this in dev_4_4. @joshmoore I don't know of a nice way to avoid the rebasing & cherry-picking above. Since all of my various branches in dev_4_4 were started and different times and don't get merged in order, when I'm rebasing onto develop they're not always going to be rebased on to the equivalent point exactly. In the case above, dev_4_4 contained #459 PR when sprint2-2ndNov branch was created on dev_4_4, but develop didn't contain the same branch when I tried to rebase onto develop. The solution here would be not to rebase this branch on develop until #459 is merged into develop. |
|
I went into the database and removed the Channel object if I remember correctly. Basically doing anything to the metadata will do it. |
|
@chris-allan Thanks, I'll give that a go. |
|
It helps you you order by id before deleting the "most recent" Channel object! But once I got the right one this test passed fine. Ready to merge. |
|
@ximenesuk: I take it then you got the nice bold red text indicating something evil had transpired? |
|
@chris-allan Yes, NPE in this case. Though the text does disappear after a second or two so you do need to pay attention when testing! |
|
Great. Definitely ready to merge. |
This is in dev_4_4 as #464. Now for develop...
NB: This PR lacks the following commit from the PR above:
e6f1968 Fix size of Jpeg Download. See #9849
because that commit has to come on top of #459, so it will be moved there.