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.

Sign up for GitHub

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

Jump to bottom

Refactor devise integration #914

Merged
merged 70 commits into from
Aug 27, 2018
Merged

Refactor devise integration #914

merged 70 commits into from
Aug 27, 2018

Conversation

artur-intech
Copy link
Contributor

@artur-intech artur-intech commented Jul 15, 2018 •
edited
Loading

Changes:

  • DB column users.password is renamed to users.plain_text_password because of a conflict with Devise
  • Users are now scoped
  • Translations and config are regenerated
  • Global customizations removed

Test:

  • Login/logout to all portals, incl. EPP and REPP (incl. Mobile-ID and ID-card)
  • Logout from one portal should not log you out from another one
  • EPP password change
  • Adding/changing new API user

Artur Beljajev added 25 commits June 19, 2018 20:00
Regenerate Devise translation
0f6df9e
Regenerate and tune Devise config
ebe831e
Remove routing specs
bb39d03
Remove rememberable and lockable modules from AdminUser
c538579
Remove unused routes
7568360
Use singular controller
28416a3
Remove Devise FailureApp
724d421
Fix links
febbe12
Revert "Remove routing specs"
450a95f 
This reverts commit bb39d03.
Remove routing specs
02137b6
Specify Devise modules for each type of user
c31f507
Refactor Devise integration
9684c8e 
- Use scoped users
- Use the named route helpers instead of hardcoded paths
Remove redirection
fa9731b
Update fixtures
904975a
Merge branch 'master' into refactor-devise-integration
e5cdb2e 
# Conflicts:
#	app/controllers/registrant/contacts_controller.rb
#	config/routes.rb
Merge branch 'master' into refactor-devise-integration
8b7769e 
# Conflicts:
#	spec/features/registrar/sign_in/mobile_id_spec.rb
Merge branch 'master' into refactor-devise-integration
d708c82
Merge branch 'regenerate-db-structure' into refactor-devise-integration
22e70e7
Rename users.password to users.plain_text_password
32ecf36 
Otherwise it conflicts with Devise
Fix tests and specs
7a054ca
Supply PaperTrail with current user
f6ad221
Extract translations
c7e426b
Fix translation
28393ad
Fix template
84bf967
Add tests
02aad0d
@artur-intech
Copy link
Contributor Author

artur-intech commented Aug 15, 2018 •
edited
Loading

@maciej-szlosarczyk The rest is old code, I only changed Devise-related things. All other things should be done separately.

@artur-intech artur-intech assigned vohmar and unassigned artur-intech Aug 15, 2018
@maciej-szlosarczyk
Copy link
Contributor

@artur-beljajev, please fix the user_for_paper_trail and let's call it a day.

@artur-intech
Copy link
Contributor Author

artur-intech commented Aug 16, 2018 •
edited
Loading

@maciej-szlosarczyk What is the point fixing PaperTrail-related minor imperfections if it's not yet clear, if we will continue using it at all? #536

@maciej-szlosarczyk
Copy link
Contributor

That issue was opened a year ago, requires infrastructure changes and is not touched since March.

I can't imagine it will become a priority within the next 12 months, given that PaperTrail generally works for us and is continuously supported. At least not bigger than auction center and other on-going developments, upgrading Rails version, Ruby version or fixing bugs and improving test coverage.

@artur-intech
Copy link
Contributor Author

artur-intech commented Aug 16, 2018 •
edited
Loading

I would still do a proper refactoring of PaperTrail part in a separate ticket. I don't see any value doing such changes now.

@maciej-szlosarczyk
Copy link
Contributor

If you say so, let's do that 👍

@vohmar
Copy link
Contributor

vohmar commented Aug 16, 2018

  • problem mentioned in Refactor devise integration #914 (comment) is still here. No Poll view in registrars' portal. According to epp logs it looks like the poll que is queried only login for the registrar in use, but not on switching the registrar (no record in log)
    image

@vohmar vohmar assigned artur-intech and unassigned vohmar Aug 16, 2018
@artur-intech
Copy link
Contributor Author

artur-intech commented Aug 17, 2018 •
edited
Loading

"No Poll view in registrars' portal"

I need additional information about the bug. It is not clear whether you see no "polls" menu on first login, or after you switch a user.

@artur-intech
Copy link
Contributor Author

artur-intech commented Aug 17, 2018 •
edited
Loading

I checked it on master, and didn't notice the behavior you described. Switching a user leads to the following screen: http://prntscr.com/kjrteg (if this is what you meant by "switching the registrar").

@vohmar
Copy link
Contributor

vohmar commented Aug 21, 2018

initial login now takes user to poll message view, but clicking on eid registrar logo takes user to this view:

/registrar

https://user-images.githubusercontent.com/7734747/44218934-6c9fcd80-a183-11e8-80e7-9c98e5c3adda.png

instead of this:

/registrar/poll

image

@artur-intech
Copy link
Contributor Author

Fixed.

@vohmar vohmar merged commit 2dd87d9 into master Aug 27, 2018
@artur-intech artur-intech mentioned this pull request Aug 27, 2018
Merged
@vohmar vohmar deleted the refactor-devise-integration branch September 6, 2018 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maciej-szlosarczyk maciej-szlosarczyk maciej-szlosarczyk approved these changes

Assignees

@artur-intech artur-intech

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@artur-intech @vohmar @teadur @maciej-szlosarczyk