Skip to content
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

Adding project on the fly when tracking donations #49

Merged
merged 7 commits into from
Dec 14, 2014
Merged

Conversation

jorgesancha
Copy link
Contributor

This PR addresses #6 and #44. In summary, it is now possible to add projects on the fly when tracking donations.

  • Replaced 'autocomplete-rails' by 'select2'
  • Added new project fields (description + url) if user chooses to add new project
  • Changed a bit how anonymous donations work - you are now redirected to the "signup" page rather than getting the "signup" form everywhere you go, which was causing some confusing UX (double sign-up form, or the donation form again, etc.)
  • Added a couple of happy-path specs for both logged in/anonymous donations.
  • Rebased all the Currency commits so that this branch can be merged automatically

There is still some work to do. We can either create new issues or commit more into this branch:

  • If something goes wrong on creation (e.g. Project description is too short), the donation form that shows up doesn't provide good error messages. It is not clear why it failed. @ferblape, I'd very much welcome your input on what's The Rails Way ™️ of doing this
  • I haven't worried about I18n this time, particularly about the 'Add project' literal being used in the coffeescript code
  • There is an ugly double-flash looking message when you TrackDon and SignUp at the same time.

Conflicts:
	Gemfile
	Gemfile.lock
@ferblape ferblape self-assigned this Dec 10, 2014
@ferblape
Copy link

Hey @jorgesancha I have merged the latest master commits in your branch fixing a couple of conflicts.

I'll take a look at what you mention about the errors later.

@jorgesancha
Copy link
Contributor Author

Thanks @ferblape , ping me on whatsapp if you want to do a hangout or sthg.

@ferblape
Copy link

I'm slowly progressing on this one, but it'll be ready for the weekend. I'm
using
http://apidock.com/rails/ActiveRecord/NestedAttributes/ClassMethods/accepts_nested_attributes_for,
which is the Rails way.

I'm also taking advantage of this PR to refactor a bit the cookies thing
and solve a couple of small bugs.

On Wed, Dec 10, 2014 at 10:02 AM, Jorge Gomez Sancha <
[email protected]> wrote:

Thanks @ferblape https://github.com/ferblape , ping me on whatsapp if
you want to do a hangout or sthg.


Reply to this email directly or view it on GitHub
#49 (comment).

@jorgesancha
Copy link
Contributor Author

Thanks Fer! I tried accepts_nested_attributes_for but had problems with strong parameters and thought maybe it wasn't for a belongs_to association. Looking fwd to see how you address it

Sent from my iPhone

On 12/12/2014, at 8:08, Fernando Blat [email protected] wrote:

I'm slowly progressing on this one, but it'll be ready for the weekend. I'm
using
http://apidock.com/rails/ActiveRecord/NestedAttributes/ClassMethods/accepts_nested_attributes_for,
which is the Rails way.

I'm also taking advantage of this PR to refactor a bit the cookies thing
and solve a couple of small bugs.

On Wed, Dec 10, 2014 at 10:02 AM, Jorge Gomez Sancha <
[email protected]> wrote:

Thanks @ferblape https://github.com/ferblape , ping me on whatsapp if
you want to do a hangout or sthg.


Reply to this email directly or view it on GitHub
#49 (comment).


Reply to this email directly or view it on GitHub.

Apart from that, a few refactors in the method that sets the parameters of the donation in the cookies when the user is not logged in
@ferblape ferblape merged commit ba8cea0 into master Dec 14, 2014
@ferblape ferblape deleted the select_2 branch December 14, 2014 10:10
@ferblape
Copy link

Merged and released!

@furilo
Copy link
Member

furilo commented Dec 16, 2014

Maybe there is something left to merge? I'm getting:

Showing /Users/alvaro/Web/trackdons/app/views/projects/new.html.erb where line #26 raised:

undefined local variable or method `autocomplete_project_name_projects_path' for #<#<Class:0x007ffc31b7de48>:0x007ffc31ea3e38>

@ferblape
Copy link

I'm afraid I broke that template. Before my changes, was the name of the project in that forma an autocomplete field?

@ferblape
Copy link

The simple fix is this one:

https://github.com/furilo/trackdons/compare/fix-projects-form?expand=1#diff-0

But I'd like to understand what was the previous code doing.

On Tue, Dec 16, 2014 at 10:17 AM, Álvaro Ortiz [email protected]
wrote:

Maybe there is something left to merge? I'm getting:

Showing /Users/alvaro/Web/trackdons/app/views/projects/new.html.erb where line #26 raised:

undefined local variable or method `autocomplete_project_name_projects_path' for #<#Class:0x007ffc31b7de48:0x007ffc31ea3e38>


Reply to this email directly or view it on GitHub
#49 (comment).

@furilo
Copy link
Member

furilo commented Dec 17, 2014

Its just from the previous gem for the autoselect, hence it can be removed.
But we would have to include there the new select-2 thing so if someone is
entering a project that already exists, he notice it.

On Wed, Dec 17, 2014 at 8:25 AM, Fernando Blat [email protected]
wrote:

The simple fix is this one:

https://github.com/furilo/trackdons/compare/fix-projects-form?expand=1#diff-0

But I'd like to understand what was the previous code doing.

On Tue, Dec 16, 2014 at 10:17 AM, Álvaro Ortiz [email protected]
wrote:

Maybe there is something left to merge? I'm getting:

Showing /Users/alvaro/Web/trackdons/app/views/projects/new.html.erb
where line #26 raised:

undefined local variable or method
`autocomplete_project_name_projects_path' for
#<#Class:0x007ffc31b7de48:0x007ffc31ea3e38>


Reply to this email directly or view it on GitHub
#49 (comment).


Reply to this email directly or view it on GitHub
#49 (comment).

http://www.mumumio.com
http://www.furilo.com

@ferblape
Copy link

Can projects be created without being logged in?

On Wed, Dec 17, 2014 at 8:56 AM, Álvaro Ortiz [email protected]
wrote:

Its just from the previous gem for the autoselect, hence it can be
removed.
But we would have to include there the new select-2 thing so if someone is
entering a project that already exists, he notice it.

On Wed, Dec 17, 2014 at 8:25 AM, Fernando Blat [email protected]
wrote:

The simple fix is this one:

https://github.com/furilo/trackdons/compare/fix-projects-form?expand=1#diff-0

But I'd like to understand what was the previous code doing.

On Tue, Dec 16, 2014 at 10:17 AM, Álvaro Ortiz [email protected]

wrote:

Maybe there is something left to merge? I'm getting:

Showing /Users/alvaro/Web/trackdons/app/views/projects/new.html.erb
where line #26 raised:

undefined local variable or method
`autocomplete_project_name_projects_path' for
#<#Class:0x007ffc31b7de48:0x007ffc31ea3e38>


Reply to this email directly or view it on GitHub
#49 (comment).


Reply to this email directly or view it on GitHub
#49 (comment).

http://www.mumumio.com
http://www.furilo.com


Reply to this email directly or view it on GitHub
#49 (comment).

@ferblape
Copy link

Instad of integrating select2, what do you think about this interaction?

new project

This is the PR: #55

@furilo
Copy link
Member

furilo commented Dec 22, 2014

Great @ferblape. Maybe we could make that the text that appears contains a link so you can go and visit the said project?

@furilo
Copy link
Member

furilo commented Dec 24, 2014

BTW, regarding if anyone can create a project, I would leave that way until we encounter user vandalism ;)

@ferblape
Copy link

Done!

Are you planning to add pictures to the projects?

On Wed, Dec 24, 2014 at 11:21 AM, Álvaro Ortiz [email protected]
wrote:

BTW, regarding if anyone can create a project, I would leave that way
until we encounter user vandalism ;)


Reply to this email directly or view it on GitHub
#49 (comment).

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

Successfully merging this pull request may close these issues.

3 participants