Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Updating comment from /mentors/applications/:id page results in 404 #960

Open
kytrinyx opened this issue Feb 28, 2018 · 54 comments
Open

Updating comment from /mentors/applications/:id page results in 404 #960

kytrinyx opened this issue Feb 28, 2018 · 54 comments

Comments

@kytrinyx
Copy link

When reviewing applications, I am able to successfully create a comment on the mentors/applications/:id page, however when attempting to update the comment it returns a 404.

@hola-soy-milk hola-soy-milk self-assigned this Mar 1, 2018
@stlireri
Copy link

stlireri commented Mar 3, 2018

@kytrinyx I would like to work on this issue

@hola-soy-milk hola-soy-milk removed their assignment Mar 3, 2018
@hola-soy-milk
Copy link
Member

Hey @stlireri, thank you so much for offering to help on this issue!

Since this is a bug that affects this year’s mentor reviewing phase, it would need to be implemented in the next few days.

Would you have time to begin looking into it this weekend? I’d be happy to help you along the way and answer any of your questions if you need help. Feel free to message or ping me!

We also have some other issues that are up for grabs if you’re interested in contributing and are worried about the time pressure 😄 In short, your help is highly appreciated!

@stlireri
Copy link

stlireri commented Mar 4, 2018

thanks @ramonh for offering to help.
Am currently having an issue with installations part. am getting a weird error.

@hola-soy-milk
Copy link
Member

Hi @stlireri, sure thing! What's the error you're getting?

@stlireri
Copy link

stlireri commented Mar 4, 2018

after running "bundle exec rails db:setup", i get this error
'Created database 'rgsocteams_development'
FATAL: role "stella_ireri" does not exist
Couldn't create database for {"adapter"=>"postgresql", "database"=>"rgsocteams_test", "host"=>"localhost", "min_messages"=>"error"}
rails aborted!
'

@kytrinyx
Copy link
Author

kytrinyx commented Mar 4, 2018

@stlireri it sounds like you need to run createuser -P -s rgsoc (from the README)

$ createuser -P -s rgsoc
Enter password for new role: rgsoc
Enter it again: rgsoc

Feel free to make the password empty (just hit enter). That's what I usually do locally unless I have production or protected data.

@hola-soy-milk
Copy link
Member

Thanks for your help @kytrinyx ! That ought'a do it

@stlireri
Copy link

stlireri commented Mar 6, 2018

Thanks alot @kytrinyx , it finally worked!
I can now access the teams app in development.

@stlireri
Copy link

stlireri commented Mar 6, 2018

Am trying to replicate the error as described by @kytrinyx but i don't seem to find the applications page. Kindly help out @kytrinyx and @ramonh.

@hola-soy-milk
Copy link
Member

Nice one! I'm glad to see you're up and running.

So if you haven't already, please go through the steps in the Quick start guide.

Take a look at the output after running the command bundle exec rake routes. The URL to have a look at the current list of applications as a mentor is /mentors/applications. You can access this locally by opening localhost:3000/mentors/applications in your browser.

Hope this steers you in the right direction! If you need further help, please let me know and good luck!

@stlireri
Copy link

stlireri commented Mar 7, 2018

when i try to navigate to that URL, i get an error stating project maintainer required . How do i go about this?

@hola-soy-milk
Copy link
Member

Hey! Right, so the problem here is that your local user isn't maintaining any accepted projects for your local season of RGSoC.

If you take a look at the mentors controller, you'll find this line:

redirect_to '/', alert: "Project maintainer required" unless current_user.project_maintainer?

This right here is what's stopping you from seeing the applications.

Let's take a look at the user model to find the method on this line:

Project.accepted.where(submitter: self).any?

Right, so we need this user to have an accepted project!

If you take a look at the project model you'll see that a project has a submitter and season it belongs to. It also has a state machine, with the default state being proposed.

So what we need is a Project submitted by you, the current user, to be accepted and assigned to the current Season.

There are a few ways to do this:

  • You can create one through the project creation page on your local instance of the teams app
  • You can create one manually by running bin/rails console and creating a Project model manually

Of course, once you do this, you'll probably need some Application records that are applying to work on this new Project! You can do this similarly.

Phew! I hope this wasn't too much at once, and that this gets you back on track! Let me know if I can help you further, please!

@stlireri
Copy link

stlireri commented Mar 8, 2018

thanks @ramonh for the analysis on the flow of the application with regard to the mentors. i now understand it better.
meanwhile, tried creating a project via the console and run the following
projects = Project.create(name: "Automation Framework", submitter_id: 33, season_id: 1, mentor_name: "tester", mentor_github_handle: "tester", mentor_email: nil, url: nil, description: "a atesting framework", issues_and_features: nil, beginner_friendly: true, aasm_state: "accepted", tags: [], source_url: nil, comments_locked: false, code_of_conduct: nil, requirements: nil, license: nil)
but when trying to save the created project by running projects.save , it returns false

@kytrinyx
Copy link
Author

kytrinyx commented Mar 8, 2018

@stlireri after it returns false, go ahead and try puts projects.errors.full_messages, that should tell you why it's failing.

@stlireri
Copy link

stlireri commented Mar 8, 2018

great! it worked, i had missed a mentors email which was a required field. Thanks

@stlireri
Copy link

Hi @kytrinyx @ramonh
i am trying to make an application using my local instance impersonating a student, which fails when i try to save a profile because of the email part. apparently no team has 2 students already so i have to add one to make the team complete.Is it possible to use a student who is already in another team?

@kytrinyx
Copy link
Author

kytrinyx commented Mar 11, 2018 via email

@hola-soy-milk
Copy link
Member

HI @stlireri, yes please! Would be good to know the error.

Furthermore, you should be able to re-assign a student from one team to another using either to console or the local teams app itself.

@hola-soy-milk
Copy link
Member

hola-soy-milk commented Mar 11, 2018

Hey @stlireri before I forget! Have a look at the db/seeds.rb file here.

There you'll find an ApplicationDraft instance that's ready to apply. Maybe you can hop on the console, set the appropriate project and see if you can manually apply it?

Hope this helps!

@stlireri
Copy link

i was able to use the application draft instance you gave at db/seeds.rb and created an application.
now i am unable to find the project the application was submitted with neither can i trace the submitter. The application had the team Team Gensync .when i try to look at the application using either of the students in the team, i get a not part of a team error.
How do i edit the same application so that i can get to it via its submitter?

@hola-soy-milk
Copy link
Member

Hey @stlireri!

Check out the application model, here you'll find that an Application instance is linked to a project.

Try hopping onto the rails console and calling that Application's project (you can also set it here if needed :) )

Hope this helps!

@stlireri
Copy link

Hi @ramonh ,
Am still unable to locate an application that's related to a project using the rails console. Am probably missing the right command to issue via the console. i tried running applications = Application.find_by_project_id(7) Application Load (0.5ms) SELECT "applications".* FROM "applications" WHERE "applications"."project_id" = $1 LIMIT $2 [["project_id", 7], ["LIMIT", 1]] but it resulted to nil .
However, from the rails app am able to see under project named Automation Framework with id = 7 , it has one application as first choice although its in draft mode. I am the project maintainer of that project but from mentors/applications its still blank.
How do i manoeuvre through all this?

@hola-soy-milk
Copy link
Member

Hey @stlireri,

Sorry, I was unclear! What I meant was taking that application and setting it's project ID to the project you wish your user is a mentor of, you know what I mean?

That is, you hop on the console and call something like the following:

> application = Application.last
> project = Project.find_by_id(7).
> application.project_id = project.id
> application.save!

Here, the validation might prevent you from saving the Application, but since we're trying to figure this out, you can skip the validations (that is, by passing { validate: false } as an option parameter to the save! call.

Hope this helps!

@stlireri
Copy link

stlireri commented Mar 15, 2018

Finally i got to the error described in the issue.

@hola-soy-milk
Copy link
Member

Yay! 🎉 Nice one

@stlireri
Copy link

The error i get when trying to update a mentors comment is No route matches [PATCH] "/mentors/comments".
From rake routes i can see that the url matching [PATCH] for mentors comment is mentors_comment_path. Again from the mentors/comments_controller.rb, the update method redirects to mentors_application_path which renders the index and show actions for mentors applications and not mentors comment.
Finally from mentors/application/comments view, the form for application comments has a specific url tag thats renders the create action only.
Kindly help me understand where i should apply the changes.

@hola-soy-milk
Copy link
Member

hola-soy-milk commented Mar 15, 2018

Nice one @stlireri! Nearly there.

Take a look at the form partial for comments. The url attribute of the form is mentors_comments_path, which isn't quite right. We want, in the case of an edit (update), that it be a different URL.

So the question is, how do we distinguish between a new and existing comment? This answer from Stack Overflow might have the answer.

Hope this helps!

@stlireri
Copy link

Hi @ramonh ,
I took a look at the stack overflow solution but i haven't implemented it yet.
So i have created a new branch from my local repository. Is that all i have to do before i can start editing the form partial for comments file locally?

@hola-soy-milk
Copy link
Member

Hey @stlireri!

Sounds good, that's right! Have you already made a fork of the repository?

@stlireri
Copy link

yeah i did fork the repository already here it is.

@hola-soy-milk
Copy link
Member

Looks good to me! You're good to go.

@stlireri
Copy link

Hi @ramonh and @kytrinyx ,
The solution given for stack overflow works for the comments partials by matching the updated comment to a PATCH.
But it still gives an error Couldn't find Mentor::Comment with 'id'=1 [WHERE "comments"."commentable_type" = $1 AND "comments"."user_id" = 33] when trying to match it up to the comment id on the mentors/comments controller.

@hola-soy-milk
Copy link
Member

Hey @stlireri,

Nice! Another step further 😄

Looks to me like there's something wrong with the params being passed along.

Ever used pry to create breakpoints? You could try dropping a binding.pry into your controller action to see what params are coming in. I suspect these could be wrongly matched up.

@stlireri
Copy link

no i haven't used pry before. I'll do install it and learn to use it along the way.

@hola-soy-milk
Copy link
Member

hola-soy-milk commented Mar 19, 2018

Hey @stlireri,

No need to install pry! It's already in the Gemfile.

If you find pry not to be helpful, you could always check the console for the PATCH request coming in and which parameters are being passed in.

@stlireri
Copy link

I think this is what is being passed as paramters
<ActionController::Parameters {"utf8"=>"✓", "_method"=>"patch", "authenticity_token"=>"wdvyNJlzYsPN9KaFxVyoeiQdXhLHywseVhseCjVj5yj9mdUPH4t9yMEHrsw+2wQBsEA49frKK0aK38mQPHecOQ==", "mentor_comment"=><ActionController::Parameters {"commentable_id"=>"1", "text"=>"This seems to be a nice team to work on the said project. !"} permitted: false>, "commit"=>"Update Comment", "controller"=>"mentors/comments", "action"=>"update", "id"=>"1"} permitted: false> --

@hola-soy-milk
Copy link
Member

hola-soy-milk commented Mar 19, 2018

Right! And looking back at the error:

Couldn't find Mentor::Comment with 'id'=1 [WHERE "comments"."commentable_type" = $1 AND "comments"."user_id" = 33]

It seems that there's no Mentor comment with id = 1 for that application. Can you confirm this in the rails console? Also, in the HTML source you should be able to see what the actual ID of the mentor comment should be.

@stlireri
Copy link

When i try to view all the comments from console here's what am getting.
[#<Comment id: 1, user_id: 1, text: "it requires knowldge on rails , ruby and git", created_at: "2018-03-08 21:01:17", updated_at: "2018-03-08 21:01:17", commentable_id: 7, commentable_type: "Project">, #<Comment id: 2, user_id: 33, text: "This seems to be a nice team to work on the said p...", created_at: "2018-03-15 20:20:31", updated_at: "2018-03-15 20:20:31", commentable_id: 1, commentable_type: "Mentor::Application">]> .
i don't seem to see any mentor id in any of them.
Am just wondering whether a mentor and a project maintainer are one and the same persons with regards to an application. Kindly clarify on this please.

@hola-soy-milk
Copy link
Member

Hi @stlireri,

You're absolutely correct. Mentors and project maintainers are the same. In this case, you have 2 comment instances. One for a project, and another for an application review. The user_id in this case is the id of the mentor.

Hope this helps!

@stlireri
Copy link

stlireri commented Mar 21, 2018

Hey @ramonh
So i have made the mentor of team Gensync to be the project maintainer. I have gone ahead and deleted the existing comment and written it a fresh but it still updating the comment throws the same error.
Couldn't find Mentor::Comment with 'id'=1 [WHERE "comments"."commentable_type" = $1 AND "comments"."user_id" = 33]
The comment
#<Comment id: 3, user_id: 33, text: "its a superb application. i love it\r\n", created_at: "2018-03-21 10:18:56", updated_at: "2018-03-21 10:18:56", commentable_id: 1, commentable_type: "Mentor::Application">

@hola-soy-milk
Copy link
Member

So it's trying to retrieve a comment with the id 1, but the comment you're updating has the id 3.

We need to pass to the action the param id=3.

I believe the reason it's passing the param id=1 is because we're inside the form for the application with id=1. You know what I mean?

@stlireri
Copy link

yeah. So how do i update the param to id=3?

@hola-soy-milk
Copy link
Member

Hmm.

Let's take another look at the form partial and the mentor comment controller.

One idea is to try and go down the road of looking at the dom element in line 1 of the form partial, or somehow including the id of the comment as a hidden element.

Notice that the update controller action is looking up the comment with ID params[:id]. This is perhaps something we need to change.

Just throwing out ideas 😄

@stlireri
Copy link

Aah niice, so from the form partial i can see the commentable_id has been set to type hidden, which is a good thing i guess.
So i think the issue is with the update controller. How do i edit that part params[ :id]? Can it be struct out completely?

@hola-soy-milk
Copy link
Member

Well the first thing to do would perhaps be to see what params the form is passing.

There are two ways to do this. You can either set up the binding.pry breakpoint I mentioned earlier or watch the rails log for the PATCH action and see what params are being passed in.

Then we should be able to see what's coming in.

One thing you could do is, like the commentable_id, pass in the comment_id as a hidden field and have the update action catch that.

But first, I would strongly suggest checking what's going on with those params 👍 Hope this helps!

@stlireri
Copy link

Hi @ramonh
Am still getting the same error Couldn't find Mentor::Comment with 'id'=1 [WHERE "comments"."commentable_type" = $1 AND "comments"."user_id" = 33].
The comment to upDATE is referencing a mentor comment with id = 1 but from console the comment linked to the application has id= 3.
kindly help.

@hola-soy-milk
Copy link
Member

hola-soy-milk commented Mar 25, 2018

Hey @stlireri!

I believe the problem comes from the fact that the id is referencing the application instance you're rating.

You need to pass the comment's id as a separate parameter. Maybe comment_id?

One solution would be to include the above comment ID as a hidden element.

@stlireri
Copy link

stlireri commented Mar 25, 2018

I have made the comment_id to be hidden but still the error persists.
Here are the params being passed
<ActionController::Parameters {"utf8"=>"✓", "_method"=>"patch", "authenticity_token"=>"XJEyumu3kMRYZumrUaNy54IIdNEkgoz4A6kBL4gJZ/5g0xWB7U+Pz1SV4eKqJN6cFlUSNhmDrKDfbda1gR0c7w==", "mentor_comment"=><ActionController::Parameters {"commentable_id"=>"1", "id"=>"3", "text"=>"its a superb application. i love it..."} permitted: false>, "commit"=>"Update Comment", "controller"=>"mentors/comments", "action"=>"update", "id"=>"1"} permitted: false>
After making comment_id to be hidden, i can see we are having two different id being passed. The former id=3 is the id belonging to the comment being updated.What could be the latter id=1 be referencing to?

@hola-soy-milk
Copy link
Member

Oh nice! So you can access that comment id with calling params[:mentor_comment][:comment_id], instead of params[:id] in the controller.

I believe the id=1 is referring to the Application you're currently reviewing. Know what I mean?

@stlireri
Copy link

Thanks @ramonh,
It finally worked. am now able to update a comment successfully. i just edited the params[:mentor_comment][:comment_id] to params[:mentor_comment][:id] and the fix worked.
Thanks for your help.

@hola-soy-milk
Copy link
Member

Hi @stlireri.

Nice one! Congrats on fixing the issue 🎉

To prevent this issue from cropping up in the future, we should make sure our tests are passing.

Are you familiar with running Rspec?

@stlireri
Copy link

stlireri commented Mar 26, 2018

I was very exited to have finally solved my first issue and forgot to run the test.
I haven't ran Rspec in a while. I'll need your help in fixing the two issues am getting after running the tests.
1) Mentors::CommentsController PUT update as a project_maintainer when comment exists redirect_to the mentor application show view Failure/Error: comment = mentor_comments.update(params[:mentor_comment][:id], update_params) and
2) Mentors::CommentsController PUT update as a project_maintainer when comment exists updates the comment Failure/Error: comment = mentor_comments.update(params[:mentor_comment][:id], update_params)

@hola-soy-milk
Copy link
Member

No probs! Happens to me all the time 😄

Have a look at the tests that aren't working. You should be able to see there which params are being passed or not passed that you need. :)

@emcoding
Copy link
Member

Update after PR #997 (Abilities):
Let's fix the comment abilities first. It should fix the access to crud actions. Then also fix the UI in case a user is not authorized.

Let's not fix it until after the User Registration Overhaul is finished.

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

No branches or pull requests

4 participants