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

Error adding user to group #1187

Closed
bbranan opened this issue Jun 1, 2017 · 13 comments
Closed

Error adding user to group #1187

bbranan opened this issue Jun 1, 2017 · 13 comments

Comments

@bbranan
Copy link
Contributor

bbranan commented Jun 1, 2017

Attempting to add a user to a group is resulting in an error.

Steps to replicate:

  1. Log in to a tenant as an admin
  2. Go to Manage Groups, select "Edit group & users" on an existing group
  3. Select the Users tab
  4. Enter the ID of a user in the tenant, click "Add"

The error displayed on the UI was You may have mistyped the address or the page may have moved.

The page where the Add button was clicked: http://testtenant.pilot.hykudirect.org/admin/groups/1/users

The page it sent me to after Add button was clicked: http://testtenant.pilot.hykudirect.org/admin/groups/1/users/add?locale=en.

It doesn't look like Honeybadger captured this issue.

According to discussion on standup call on 6/1, the user ID field should not be displayed in the UI. The process of adding a user to a group should be:

  • On the Users tab, type in the Search for User field
  • Select the user that is displayed from the search result and click Add
@mjgiarlo
Copy link
Member

mjgiarlo commented Jun 6, 2017

This should have been resolved with #839, but I can now confirm that I am seeing the buggy behavior in AWS, docker-compose, and local deployments when RAILS_ENV=production. The JavaScript that is supposed to fire is in the compiled application.js, so the code is there, and I see no errors in the JavaScript console.

@mjgiarlo mjgiarlo removed their assignment Jun 6, 2017
@jcoyne jcoyne removed the dashboard label Jun 12, 2017
@dazza-codes dazza-codes self-assigned this Jun 14, 2017
@dazza-codes
Copy link
Contributor

dazza-codes commented Jun 14, 2017

Trying to replicate this in my hyku tenant at https://{xxx}.demo.hydrainabox.org/

Steps to replicate:

  1. Log in to a tenant as an admin
  2. Go to Dashboard
  3. Find the "CONFIGURATION" group in the sidebar and select "Users and groups"
  4. Choose the "Manage groups" in the sidebar and create a new group
  5. Once created, choose the "Edit group and users" under "Actions"
  6. Select the Users tab
  7. While trying to search for a user, it redirected to the user search results and there was no way to get back; when using the browser back button, all the UI CSS got completely messed up. (But that's a different issue.) OK, so create a new user (non-admin) and/or get a User-ID and go back to the dashboard to start over.
  8. Enter the ID of a user in the tenant, click "Add"
  • this fails and hits a 500 page
  • the redirect page is https://{xxx}.demo.hydrainabox.org/admin/groups/1/users/add?locale=en
  • there seems to be no {user-id} param in that URL

It seems like the Search box is not very well integrated with the User-ID field. The Search -> Go button redirects to the search results and completely removes the user from the manage group UI. It seems like the Search -> Go button could open a modal dialog to display search results or we should just remove that button or have that page open in a new browser window? Actually, it seems like a better design would be to have the User-ID field as a search drop-down/auto-complete select field and remove everything else (Search field, and the associated Go button, and the Add button).

@mjgiarlo
Copy link
Member

@darrenleeweber If you spin up Hyku in development mode locally, you can see how it's supposed to look. The issue is that some hidden fields are not being hidden. See #839 (fixed #837) for prior work on this.

@mjgiarlo
Copy link
Member

AFAICT, this only fails in production mode, and that's key to this work.

@dazza-codes
Copy link
Contributor

dazza-codes commented Jun 14, 2017

Confirmed that everything behaves as expect in dev-env.

Using firebug inspector, the div markup details are slightly different in prod vs. dev, i.e.

  • prod
<div class="panel-body">
    <form class="simple_form form-inline pull-left js-group-user-search" 
          action="/users?locale=en"
          accept-charset="UTF-8" method="get">
    <form class="simple_form form-inline pull-left js-group-user-add"
          action="/admin/groups/1/users/add?locale=en"
          accept-charset="UTF-8" method="post">
</div>
  • dev
<div class="panel-body">
    <form class="simple_form form-inline pull-left js-group-user-search" 
          action="/users?locale=en" 
          accept-charset="UTF-8" method="get">
    <form class="simple_form form-inline pull-left js-group-user-add" 
          action="/admin/groups/1/users/add?locale=en" 
          accept-charset="UTF-8" method="post" style="display: none;">
</div>

So the only difference in this markup is that dev has the user-add form hidden.

@dazza-codes
Copy link
Contributor

dazza-codes commented Jun 14, 2017

Files involved in this form include:

$ git grep -l 'js-group-user-add'
app/assets/javascripts/hyku/groups/add_member.js
app/views/admin/groups/users.html.erb
spec/features/admin_dashboard_spec.rb
spec/views/admin/groups/users.html.erb_spec.rb

The JS code is in https://github.com/samvera-labs/hyku/blob/master/app/assets/javascripts/hyku/groups/add_member.js#L2

@dazza-codes
Copy link
Contributor

dazza-codes commented Jun 15, 2017

Struggled to get a prod-env running on my laptop to debug this problem. Documented my solution in the wiki at https://github.com/samvera-labs/hyku/wiki/Hyku-Development-Guide#production-debugging-on-laptop and I can now replicate this issue on my laptop.

TIP: The chrome developer tools can pretty-print the compiled application.js file and then allow a find to locate the relevant JS function (using a find for js-group-user-add found it at line 48604!).

@dazza-codes
Copy link
Contributor

This might be a red-herring, but the chrome JS console reports an error in the compiled application.js file:

Uncaught Error: See almond README: incorrect module build, no module name
    at define (application-b2f35d1….js:24)
    at application-b2f35d1….js:29
    at application-b2f35d1….js:29
define @ application-b2f35d1….js:24
(anonymous) @ application-b2f35d1….js:29
(anonymous) @ application-b2f35d1….js:29

I don't know what impact this is having on the performance of the JS script(s).

@jcoyne
Copy link
Member

jcoyne commented Jun 15, 2017

I'm pretty certain that's a red herring.

@dazza-codes
Copy link
Contributor

dazza-codes commented Jun 15, 2017

There's a JS error in Hyrax.popovers(), i.e.

    popovers: function() {
        $("a[data-toggle=popover]").popover({  // this call to .popover is not a function
            html: !0
        }).click(function() {
            return !1
        })
    },

This is called from

Blacklight.onLoad(function() {
    Hyrax.initialize()
})

The JS stacktrace in the chrome console is a bit cryptic in the compressed JS file, i.e.

Uncaught TypeError: $(...).popover is not a function
    at Object.popovers (application-b2f35d1….js:formatted:35266)
    at Object.initialize (application-b2f35d1….js:formatted:35230)
    at application-b2f35d1….js:formatted:35321
    at Object.activate (application-b2f35d1….js:formatted:10867)
    at HTMLDocument.<anonymous> (application-b2f35d1….js:formatted:10877)
    at HTMLDocument.dispatch (application-b2f35d1….js:formatted:2629)
    at HTMLDocument.g.handle (application-b2f35d1….js:formatted:2561)
    at Object.t.dispatch (application-b2f35d1….js:formatted:8806)
    at n.t.Controller.n.notifyApplicationAfterPageLoad (application-b2f35d1….js:formatted:10309)
    at n.t.Controller.n.visitCompleted (application-b2f35d1….js:formatted:10340)

This JS from Hyrax.popovers() must be in the hyrax gem, from

@jcoyne
Copy link
Member

jcoyne commented Jun 15, 2017

@darrenleeweber that should be from bootstrap popovers: http://getbootstrap.com/javascript/#popovers That may be missing from the javascript manifest.

@jcoyne
Copy link
Member

jcoyne commented Jun 15, 2017

@dazza-codes
Copy link
Contributor

dazza-codes commented Jun 15, 2017

Clueless about how to solve this exactly, because it appears the compressed application.js file does contain some bs.popover code. It's possible there is a conflict in the included JS libs that only arises when the JS files are consolidated into one large file and compressed.
Related SO question:

I have to throw back this issue because I don’t know how to debug and fix it. The JS code in production is obscure, difficult to debug and I haven't found any way to get more information about how the JS versions are packaged by the asset pipeline.

I recommend that hyrax/hyku try to adopt new JS packaging options, see

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

No branches or pull requests

5 participants