Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

bring back the takes UI #4077

Merged
merged 31 commits into from
Aug 9, 2016
Merged

bring back the takes UI #4077

merged 31 commits into from
Aug 9, 2016

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Jul 7, 2016

#3994

#4099 (diff)

Refactor the new takes/membership code now that #4023 and #4024 are merged into this new line of development.

This was referenced Jul 7, 2016
@chadwhitacre
Copy link
Contributor Author

Page is barely loading! 😱

screen shot 2016-07-07 at 12 20 12 pm

@chadwhitacre
Copy link
Contributor Author

Yay! 😍

screen shot 2016-07-07 at 2 03 41 pm

@chadwhitacre
Copy link
Contributor Author

Alright, I migrated client-side tests to Splinter in #4079, and have inserted that into the PR chain. Ready to write some tests for this UI! :-)

@chadwhitacre
Copy link
Contributor Author

#4076 is in, and I've rebased on master. Onward!

@chadwhitacre
Copy link
Contributor Author

BLAM!!!!!!!!!!!!!!!!!!!

Adding members, tested ... 👍

screen shot 2016-07-14 at 12 11 01 pm
screen shot 2016-07-14 at 12 10 52 pm

@chadwhitacre
Copy link
Contributor Author

As part of getting this to work, I made a couple other changes that should be separate PRs.

@chadwhitacre
Copy link
Contributor Author

2af9e57 2378e26 f3dc415 could/should be cherry-picked into separate PRs.

@chadwhitacre
Copy link
Contributor Author

Hmm ... a test failure to investigate. Perhaps related to PhantomJS vs. Chrome?

@chadwhitacre
Copy link
Contributor Author

Okay! Travis is happy. 👍

Now to bring removing users, and then setting takes ...

@chadwhitacre
Copy link
Contributor Author

ba81708 and 6d190c5 can go in a separate PR as well.

@chadwhitacre
Copy link
Contributor Author

... aaaaaaaand 1ea1294. :)

@ghost
Copy link

ghost commented Jul 20, 2016

You won't need lookup.json anymore? Otherwise, I can make the small adjustments we talked about (~ don't return the non-searchable users except in the case of an exact match).

@chadwhitacre
Copy link
Contributor Author

You won't need the lookup.json anymore? Otherwise, I can make the small adjustments we talked about (~ don't return the non-searchable users except in the case of an exact match).

We should address that before going live with the rest of the work on this PR, yes. Wanna do that in a separate PR, and we can be sure to deploy that before landing this PR?

@ghost
Copy link

ghost commented Jul 20, 2016

I'm getting a bunch of errors while trying to run only one test. I'll finish the tests and make the PR after lunch :-)

@chadwhitacre
Copy link
Contributor Author

I'm getting a bunch of errors while trying to run only one test. I'll finish the tests and make the PR after lunch :-)

If you start a PR with the broken code, we can maybe help debug. :)

@chadwhitacre
Copy link
Contributor Author

(You could be hitting problems introduced by #4079.)

@ghost
Copy link

ghost commented Jul 20, 2016

No broken code, but I think I'll add a Makefile rule to be able to only run a batch of tests (pytest's -k option).
The PR is alive in #4090!

@chadwhitacre
Copy link
Contributor Author

I think I'll add a Makefile rule to be able to only run a batch of tests (pytest's -k option).

Go for it. I have this in my .profile:

alias envtest="honcho run -e defaults.env,local.env,tests/test.env,tests/local.env py.test"

So I run tests like this:

envtest tests/ttw/test_foo.py -k test_foo_

@chadwhitacre chadwhitacre changed the title Preen takes bring back the takes UI Jul 20, 2016
chadwhitacre and others added 25 commits August 9, 2016 10:10
IDs are brittle, I guess? Classes feel more betterer.
@Nashe points out that we only allow POST in the first place.

#4077 (comment)
The team navbar's behaviour was broken since some months, it's now
fixed ans some tests were added.
Make it available only to the administrators, owners and members of the
team.
Maybe got caught in a global search/replace?
This gives us more control over the UX ... and helps us dodge difficulty
with browser testing.
In the previous commit, we stopped using the browser's native confirm
dialog.
- remove redundant test class
- aim for one assert per test case
@chadwhitacre
Copy link
Contributor Author

💃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants