HR Applications: Improvements

PerMalmberg commented 8 years ago

Here are a few requests/questions from our recruiters:

Show date/time of when an application was submitted.
Is it possible to require an account-wide API, i.e. reject APIs that are only for one character?
Allow an arbitrary number of APIs to be entered when creating an application.
Make text in edit boxes copyable in applications - in Firefox nothing can be selected (only shows "stop"-mouse pointer. Chrome displays the same pointer, but text can be selected).
Is is possible to add functionality to add/remove custom questions to the application form? "Are you a spy?" isn't of much use.

Thoughts?

Adarnof commented 8 years ago

Blows my mind that info isn't shown. Totally thought it was. It will be.
see 3
Addressed in #300 - specifically all APIs on the account are available to recruiters with a new permission. That being said, users who do not fall in the blue/member category can enter APIs of any mask, so that validation will need to change. "If not blue, require member mask" should do the trick.
For some unknown reason the page was originally designed as an uneditable form, the way to do that in html is to disable the fields. That could easily become a table in a template or something more friendly. I've become quite fond of bootstrap's panels lately: a question header and answer body would look beautiful. Maybe even accordion to get fancy.
That's a really big project. I have two ideas:
- new models, ApplicationQuestion and ApplicationAnswer. You'd define questions, the form would be dynamically rendered as per this example and user answers matched to questions at the database level.
- fuck it, store everything flat. You'd define questions as before, but this time Q+A get stored in a serialized dict. ~~Not sure how the DB will like it though, that'll be one hell of a charfield.~~ EDIT: TextField is a thing. But very very tolerant to changes and very easy to render: for question, answer in answerdict in a template is not complicated.

Thoughts?

Adarnof commented 8 years ago

To keep my sanity, I'll write down the application design I've come up with.

ApplicationQuestion

has a title and optional help text
all of them accept text. there's really no way of implementing other question types, sorry. If you want a "pick one of" just say that in help text, but no enforcement can occur

ApplicationForm

points to a corp, contains questions

Application

points to a user and form
other things like reviewer, approved/denied, etc

ApplicationResponse

points to a question and an application
contains response text

ApplicationComment

points to an application and commenting user
contains comment text

Simple, yes?

To build an application, you'd start with the questions. Then you'd package them together with a corp in a form.

PerMalmberg commented 8 years ago

Sounds good, but do you medan that you'd manually create the form or will there be a gui for it? Even the django admin backend would do as a gui I think.

Adarnof commented 8 years ago

This would happen in the django admin site.

PerMalmberg commented 8 years ago

That is perfectly acceptable!

Adarnof commented 8 years ago

@PerMalmberg If you're looking for something to do, the hrapp branch is ready for testing.

PerMalmberg commented 8 years ago

Sweet, I'll set this up tonight and let my recruiters have a go at it.

Aelonius commented 8 years ago

I've ran a short test, but the API's are still locked behind the one who is reviewing it. As a result, if you mark it as in progress others can not see the assigned API's. The reject/approve buttons work flawlessly, but I really need my recruiters to be able to see the applications that others are taking lead on, in case they need to take over.

Adarnof commented 8 years ago

No they are not. The only test for displaying APIs is a check for the view_apis permission here and here, and I just noticed the first one is ignored. Can you triple-check you assigned the permission? Maybe reload apache?

Aelonius commented 8 years ago

I've used the method that you evemailed me before (git fetch --all) and then update it. http://puu.sh/nGgGW/b5ab2826af.png

Now I might be totally bad but nothing points out to any errors, except:

Cleaning up...
Creating tables ...
Installing custom SQL ...
Installing indexes ...
Installed 0 object(s) from 0 fixture(s)
No evolution required.

Almost looks to me like it didn't update the database? (Again newb here with linux)

Adarnof commented 8 years ago

Did you check out the hrapp branch? git checkout hrapp

Aelonius commented 8 years ago

Ah that must've been it. BRB doing it again :p

Aelonius commented 8 years ago

Alright,

I've got it up right now and I'll update this post with some things I notice.

All old applications will not work due to the extensive rework of the database. That means I can not read the prior applications for now. I'll update soon after adding forms.
The application form designer should have some preset field for API data so JackKnife can link to that. Right now not sure how it'll tell jackknife which API is relevant for them?

Adarnof commented 8 years ago

The models still exist so you could still read them from the admin site if you re-register the model in hrapplications/models.py

Is there interest in a conversion script? That could be achieved.

Aelonius commented 8 years ago

I think if you're going to finalize the changes that a conversion script would be very much preferred. Question is how you'll do that effectively then.

Adarnof commented 8 years ago

Well it's pretty simple. The old applications have pre-defined questions so I can easily create ApplicationQuestion models for them. I would then have to construct a set of all corps there are applications for, and create ApplicationForm models for each corp containing the questions I just created.

Then from each old application I'd create a new Application for the user to the corp form. Finally I'd create ApplicationResponse models for each question in the old forms for each application.

Easy, no?

Only downside I see is that every corp who had previously received an application is now automatically accepting them again, and to stop accepting them means deleting all the old applications. Makes a good argument for an enabled boolean on the ApplicationForm models.

PerMalmberg commented 8 years ago

Is there an easy way to try out branches, without setting up a second server, or doing the whole backup-test-restore dance?

Adarnof commented 8 years ago

Here's a completely untested demo script that may or may not work.

Usage: save as a .py file somewhere, then python manage.py shell < my_script.py

Adarnof commented 8 years ago

@PerMalmberg short answer: no.

Long answer, depends on branch and its changes. This one is adding new models and not touching the old ones nor adjusting settings, so it's pretty safe to try out. That being said, if the models change, you may have to do some SQL wizardry (drop table x) before the final release. I'll try to avoid breaking evolutions.

PerMalmberg commented 8 years ago

Mm, thought as much. I'll do a copy of the VM and run it on that.

PerMalmberg commented 8 years ago

I'm getting a 500 error when trying to add a new question, but I can't find any trace of the error in the logs ?!

The url is /admin/hrapplications/applicationquestion/add/?_popup=1

PerMalmberg commented 8 years ago

It's swedish characters that causes the problem. "åäö" in the text field breaks it.

PerMalmberg commented 8 years ago

Also, I don't quite see how the questions relate to the corp selected in /admin/hrapplications/applicationform/1/ Is the intention that it should be possible to add forms for the different corporations? If so that seems broken as I could create a new form with corp X selected, and delete the same from with corp Y selected...or perhaps it removed both forms? Can't really say as Iäm not sure how it should work.

Adarnof commented 8 years ago

Huh?

Adarnof commented 8 years ago

https://github.com/R4stl1n/allianceauth/wiki/HRApplications

PerMalmberg commented 8 years ago

Will have another go at it tomorrow.

Adarnof commented 8 years ago

Sounds good.

Aelonius commented 8 years ago

A very practical question Adarnof (Thanks for wicked work btw). How do we add jackknife-capable API fields to the form?

Adarnof commented 8 years ago

Excellent question. What does that mean?

Aelonius commented 8 years ago

Well.

I am building a form right now for my corporation and I am not sure based on the interface you put in the test, how I can add the API fields so that on the checks I can click to jack knife.

Adarnof commented 8 years ago

Oh I see.

APIs are automatically pulled from their user account. I didn't write in support for API fields explicitly.

Adarnof commented 8 years ago

On that note, there's no API validation of any kind. How about a toggle-able field in the form which defines API requirements, either MEMBER BLUE or None

PerMalmberg commented 8 years ago

I've set up a separate server that our recruiters will play around on. I'll get back with a summary of their feedback.

PerMalmberg commented 8 years ago

@Adarnof Initially it looks good, two issues:

Require APIs to be entered before a new corporation application can be created. This is rather important.
One of my helpers has managed to get into a situation where every time he tries to create a new application, he ends up with the following error:

IntegrityError at /hr_application_create/1
(1062, "Duplicate entry '1' for key 'form_id'")
Request Method: POST
Request URL:    http://serveraddress/hr_application_create/1 
Django Version: 1.6.5
Exception Type: IntegrityError
Exception Value:    
(1062, "Duplicate entry '1' for key 'form_id'")
Exception Location: /usr/local/lib/python2.7/dist-packages/MySQLdb/connections.py in defaulterrorhandler, line 36
Python Executable:  /usr/bin/python
Python Version: 2.7.6
Python Path:    
['/home/allianceauth/allianceauth',
 '/usr/lib/python2.7',
 '/usr/lib/python2.7/plat-x86_64-linux-gnu',
 '/usr/lib/python2.7/lib-tk',
 '/usr/lib/python2.7/lib-old',
 '/usr/lib/python2.7/lib-dynload',
 '/usr/local/lib/python2.7/dist-packages',
 '/usr/lib/python2.7/dist-packages']

This only happens on his particular test account, my test-account works just fine (can delete and create new applications). I've also logged into his account and I get the same issue.

Adarnof commented 8 years ago

Rogers, API requirements will be added.

And that issue is because I'm a dumb and used the wrong field type. Right now there can only be one application to any given corp at the same time. Whoops.

Adarnof commented 8 years ago

eaa44c02548c86d66ef00be0a2495e84ed57f009

PerMalmberg commented 8 years ago

Nope, still same issue if there is one existing application. (yes, did a git pull and ran update.sh and restarted apache)

Adarnof commented 8 years ago

manually run python manage.py evolve --hint --execute - does it try to do anything?

PerMalmberg commented 8 years ago

Yeah, it tries to. I didn't see this in the output when I ran update.sh

ERROR:root:Unexpected error: 'field_type'
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/management/commands/evolve.py", line 65, in handle
    self.evolve(*app_labels, **options)
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/management/commands/evolve.py", line 125, in evolve
    sql.extend(self.evolve_app(app))
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/management/commands/evolve.py", line 146, in evolve_app
    hinted_evolution = self.diff.evolution()
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/diff.py", line 281, in evolution
    ATTRIBUTE_DEFAULTS[prop])
KeyError: 'field_type'
Traceback (most recent call last):
  File "manage.py", line 10, in 
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/__init__.py", line 399, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/__init__.py", line 392, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/base.py", line 242, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/local/lib/python2.7/dist-packages/django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/management/commands/evolve.py", line 65, in handle
    self.evolve(*app_labels, **options)
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/management/commands/evolve.py", line 125, in evolve
    sql.extend(self.evolve_app(app))
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/management/commands/evolve.py", line 146, in evolve_app
    hinted_evolution = self.diff.evolution()
  File "/usr/local/lib/python2.7/dist-packages/django_evolution/diff.py", line 281, in evolution
    ATTRIBUTE_DEFAULTS[prop])
KeyError: 'field_type'

Adarnof commented 8 years ago

Oh boy. Evolutions is broken. Have I ever mentioned just how much I hate it?

You'll have to drop some tables: hrapplications_hrapplication, django_evolution, django_project_version then re-sync the database. May have to drop hrapplications_applicationcomment as well because of foreign keys.

Unless you don't mind wiping the test server DB entirely and starting over? That's much easier.

PerMalmberg commented 8 years ago

I'll try dropping first so the users don't have to re-register.

PerMalmberg commented 8 years ago

After having dropped tables and resynched the db, the error still remains.

Adarnof commented 8 years ago

Check to see if you managed to pull the changes - your hrapplications/models.py file line 24 should be a ForeignKey

PerMalmberg commented 8 years ago

Yup, its there.

Adarnof commented 8 years ago

Does this user already have applications?

PerMalmberg commented 8 years ago

No, I've deleted the only application was created, I'll drop all the application tables and try again.

PerMalmberg commented 8 years ago

Nah, this isn't working. Got the evolution error again. Restart from scratch?

Adarnof commented 8 years ago

Unfortunately sounds like that's the only option. Drop database, create it again.

PerMalmberg commented 8 years ago

Ok, I'll make a new copy of the production VM and restart tomorrow.

Adarnof commented 8 years ago

No need to copy the VM - just wipe the database

allianceauth / allianceauth

HR Applications: Improvements #305