-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix application_question.locator for payload #227
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1st Review
@@ -18,10 +18,13 @@ | |||
cron: '0 1 * * *' # Runs once per day (at 1 am) | |||
class: Updater::ExistingCompanyJobsJob | |||
queue: updates | |||
devitjobs_updater: | |||
enabled: <%= Rails.env.production? %> # production only as it interferes with end-to-end testing | |||
devitjobs_updater: # off temporarily as interferes with end-to-end testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this interferes and revert this in due course
config/sidekiq.yml
Outdated
cron: '0 2 * * *' # Runs once per day (at 2 am) | ||
class: Importer::Api::DevitJobsJob | ||
queue: updates | ||
enabled: <%= Rails.env.production? %> # production only as it interferes with end-to-end testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly - why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FormFiller runs as a background job. Can't run if there's three hundred background jobs already in the queue. Also it's just annoying in development to have the database constantly being filled with jobs I didn't seed.
lib/tasks/testing.rake
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting approach - can we add some comments for how this should work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is exactly what I want
|
||
RSpec.feature 'ApplyToJob', type: :feature, apply_to_job: true do | ||
before do | ||
skip 'No URL available to test' if ENV['URL_FOR_TESTING'].nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we put the url for testing in the ENV file? Seems like an odd place to store this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's temporary memory, not permanent storage. To my knowledge, it's the best way to pass a variable from a rake task to an rspec file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2nd Review
return photo if question.photo? | ||
return cover_letter if question.cover_letter? | ||
|
||
resume |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come you've set it up like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An application form has several attachments now ... and the number will probably grow.
I need to be able to retrieve that specific attachment.
I need the question as a parameter to be able to do so.
@@ -42,14 +42,13 @@ def fetch_application_question_set | |||
# i.e : autocomplete & mandatory location | |||
# Returns true if no such question is found, indicating the user can apply with Cheddar. | |||
def apply_with_cheddar | |||
return false unless form_structure.dig(:core_questions, :questions) | |||
return false unless form_structure.dig('core_questions', 'questions') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we accessing off strings rather than symbols now? What is the rule we're applying in general?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I think i am loosing sym while saving into the database. It is fix because we are in a sprint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file doing?
@@ -37,7 +37,7 @@ | |||
] | |||
}, | |||
{ | |||
"attribute": "where_are_you_based_or_where_would_you_be_planning_on_workin", | |||
"attribute": "where-are-you-based-or-where-would-you-be-planning-on-workin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why hyphen rather than underscore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick fix yesterday. I need to correct it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3rd Review
@@ -5,6 +5,8 @@ def get_application_question_set(_job, data) | |||
formatted_data = Importer::BamboohrFieldsFormatter.call(data.with_indifferent_access) | |||
Importer::FieldsBuilder.call(formatted_data) | |||
end | |||
|
|||
STATE_FIELDS = [{ id: "162", label: "Aberdeen City" }, { id: "161", label: "Aberdeenshire" }, { id: "165", label: "Angus" }, { id: "167", label: "Ards" }, { id: "163", label: "Argyll and Bute" }, { id: "178", label: "Ballymena" }, { id: "179", label: "Ballymoney" }, { id: "181", label: "Banbridge" }, { id: "171", label: "Barking and Dagenham" }, { id: "182", label: "Barnet" }, { id: "184", label: "Barnsley" }, { id: "169", label: "Bath and North East Somerset" }, { id: "115", label: "Bedford" }, { id: "174", label: "Belfast" }, { id: "116", label: "Berkshire" }, { id: "173", label: "Bexley" }, { id: "177", label: "Birmingham" }, { id: "170", label: "Blackburn with Darwen" }, { id: "186", label: "Blackpool" }, { id: "176", label: "Blaenau Gwent" }, { id: "185", label: "Bolton" }, { id: "180", label: "Bournemouth" }, { id: "187", label: "Bracknell Forest" }, { id: "188", label: "Bradford" }, { id: "172", label: "Brent" }, { id: "175", label: "Bridgend" }, { id: "183", label: "Brighton and Hove" }, { id: "190", label: "Bristol, City of" }, { id: "189", label: "Bromley" }, { id: "117", label: "Buckinghamshire" }, { id: "191", label: "Bury" }, { id: "192", label: "Caerphilly" }, { id: "199", label: "Calderdale" }, { id: "118", label: "Cambridgeshire" }, { id: "202", label: "Camden" }, { id: "205", label: "Cardiff" }, { id: "203", label: "Carmarthenshire" }, { id: "197", label: "Carrickfergus" }, { id: "207", label: "Castlereagh" }, { id: "193", label: "Central Bedfordshire" }, { id: "194", label: "Ceredigion" }, { id: "119", label: "Cheshire" }, { id: "196", label: "Cheshire East" }, { id: "160", label: "Cheshire West and Chester" }, { id: "200", label: "Clackmannanshire" }, { id: "201", label: "Coleraine" }, { id: "208", label: "Conwy" }, { id: "198", label: "Cookstown" }, { id: "120", label: "Cornwall" }, { id: "166", label: "County Antrim" }, { id: "168", label: "County Armagh" }, { id: "216", label: "County Down" }, { id: "229", label: "County Fermanagh" }, { id: "368", label: "County Londonderry" }, { id: "369", label: "County Tyrone" }, { id: "204", label: "Coventry" }, { id: "195", label: "Craigavon" }, { id: "206", label: "Croydon" }, { id: "121", label: "Cumbria" }, { id: "209", label: "Darlington" }, { id: "210", label: "Denbighshire" }, { id: "211", label: "Derby" }, { id: "122", label: "Derbyshire" }, { id: "217", label: "Derry" }, { id: "123", label: "Devon" }, { id: "124", label: "Didcot" }, { id: "214", label: "Doncaster" }, { id: "125", label: "Dorset" }, { id: "218", label: "Dudley" }, { id: "213", label: "Dumfries and Galloway" }, { id: "215", label: "Dundee City" }, { id: "212", label: "Dungannon" }, { id: "126", label: "Durham" }, { id: "219", label: "Ealing" }, { id: "220", label: "East Ayrshire" }, { id: "222", label: "East Dunbartonshire" }, { id: "223", label: "East Lothian" }, { id: "226", label: "East Renfrewshire" }, { id: "227", label: "East Riding of Yorkshire" }, { id: "127", label: "East Sussex" }, { id: "221", label: "Edinburgh, City of" }, { id: "224", label: "Eilean Siar" }, { id: "225", label: "Enfield" }, { id: "128", label: "Essex" }, { id: "228", label: "Falkirk" }, { id: "230", label: "Fife" }, { id: "231", label: "Flintshire" }, { id: "232", label: "Gateshead" }, { id: "233", label: "Glasgow City" }, { id: "129", label: "Gloucestershire" }, { id: "370", label: "Greater London" }, { id: "138", label: "Greater Manchester" }, { id: "234", label: "Greenwich" }, { id: "235", label: "Gwynedd" }, { id: "238", label: "Hackney" }, { id: "236", label: "Halton" }, { id: "241", label: "Hammersmith and Fulham" }, { id: "131", label: "Hampshire" }, { id: "245", label: "Haringey" }, { id: "244", label: "Harrow" }, { id: "243", label: "Hartlepool" }, { id: "237", label: "Havering" }, { id: "132", label: "Herefordshire" }, { id: "158", label: "Hertfordshire" }, { id: "240", label: "Highland" }, { id: "239", label: "Hillingdon" }, { id: "242", label: "Hounslow" }, { id: "248", label: "Inverclyde" }, { id: "164", label: "Isle of Anglesey" }, { id: "246", label: "Isle of Wight" }, { id: "362", label: "Isles of Scilly" }, { id: "247", label: "Islington" }, { id: "249", label: "Kensington and Chelsea" }, { id: "133", label: "Kent" }, { id: "250", label: "Kingston upon Hull" }, { id: "252", label: "Kingston upon Thames" }, { id: "251", label: "Kirklees" }, { id: "253", label: "Knowsley" }, { id: "254", label: "Lambeth" }, { id: "134", label: "Lancashire" }, { id: "260", label: "Larne" }, { id: "256", label: "Leeds" }, { id: "255", label: "Leicester" }, { id: "135", label: "Leicestershire" }, { id: "136", label: "Leinster" }, { id: "257", label: "Lewisham" }, { id: "259", label: "Limavady" }, { id: "137", label: "Lincolnshire" }, { id: "261", label: "Lisburn" }, { id: "258", label: "Liverpool" }, { id: "130", label: "London, City of" }, { id: "262", label: "Luton" }, { id: "265", label: "Magherafelt" }, { id: "264", label: "Medway" }, { id: "363", label: "Merseyside" }, { id: "271", label: "Merthyr Tydfil" }, { id: "269", label: "Merton" }, { id: "263", label: "Middlesbrough" }, { id: "159", label: "Middlesex" }, { id: "267", label: "Midlothian" }, { id: "266", label: "Milton Keynes" }, { id: "268", label: "Monmouthshire" }, { id: "270", label: "Moray" }, { id: "272", label: "Moyle" }, { id: "282", label: "Neath Port Talbot" }, { id: "276", label: "Newcastle upon Tyne" }, { id: "284", label: "Newham" }, { id: "285", label: "Newport" }, { id: "287", label: "Newry and Mourne" }, { id: "281", label: "Newtownabbey" }, { id: "139", label: "Norfolk" }, { id: "273", label: "North Ayrshire" }, { id: "274", label: "North Down" }, { id: "275", label: "North East Lincolnshire" }, { id: "278", label: "North Lanarkshire" }, { id: "279", label: "North Lincolnshire" }, { id: "280", label: "North Somerset" }, { id: "283", label: "North Tyneside" }, { id: "286", label: "North Yorkshire" }, { id: "140", label: "Northamptonshire" }, { id: "367", label: "Northern Ireland" }, { id: "141", label: "Northumberland" }, { id: "277", label: "Nottingham" }, { id: "142", label: "Nottinghamshire" }, { id: "288", label: "Oldham" }, { id: "289", label: "Omagh" }, { id: "290", label: "Orkney Islands" }, { id: "143", label: "Oxfordshire" }, { id: "291", label: "Pembrokeshire" }, { id: "292", label: "Perth and Kinross" }, { id: "297", label: "Peterborough" }, { id: "293", label: "Plymouth" }, { id: "294", label: "Poole" }, { id: "295", label: "Portsmouth" }, { id: "296", label: "Powys" }, { id: "302", label: "Reading" }, { id: "301", label: "Redbridge" }, { id: "298", label: "Redcar and Cleveland" }, { id: "303", label: "Renfrewshire" }, { id: "300", label: "Rhondda, Cynon, Taff" }, { id: "304", label: "Richmond upon Thames" }, { id: "299", label: "Rochdale" }, { id: "305", label: "Rotherham" }, { id: "144", label: "Rutland" }, { id: "314", label: "Salford" }, { id: "306", label: "Sandwell" }, { id: "308", label: "Scottish Borders, The" }, { id: "309", label: "Sefton" }, { id: "311", label: "Sheffield" }, { id: "353", label: "Shetland Islands" }, { id: "145", label: "Shropshire" }, { id: "315", label: "Slough" }, { id: "318", label: "Solihull" }, { id: "146", label: "Somerset" }, { id: "307", label: "South Ayrshire" }, { id: "147", label: "South Glamorgan" }, { id: "310", label: "South Gloucestershire" }, { id: "316", label: "South Lanarkshire" }, { id: "326", label: "South Tyneside" }, { id: "364", label: "South Yorkshire" }, { id: "323", label: "Southampton" }, { id: "319", label: "Southend-on-Sea" }, { id: "329", label: "Southwark" }, { id: "312", label: "St. Helens" }, { id: "148", label: "Staffordshire" }, { id: "322", label: "Stirling" }, { id: "313", label: "Stockport" }, { id: "325", label: "Stockton-on-Tees" }, { id: "321", label: "Stoke-on-Trent" }, { id: "320", label: "Strabane" }, { id: "149", label: "Suffolk" }, { id: "317", label: "Sunderland" }, { id: "150", label: "Surrey" }, { id: "324", label: "Sutton" }, { id: "327", label: "Swansea" }, { id: "328", label: "Swindon" }, { id: "330", label: "Tameside" }, { id: "331", label: "Telford and Wrekin" }, { id: "332", label: "Thurrock" }, { id: "333", label: "Torbay" }, { id: "334", label: "Torfaen" }, { id: "336", label: "Tower Hamlets" }, { id: "335", label: "Trafford" }, { id: "366", label: "Tyne and Wear" }, { id: "337", label: "Vale of Glamorgan, The" }, { id: "342", label: "Wakefield" }, { id: "343", label: "Walsall" }, { id: "340", label: "Waltham Forest" }, { id: "346", label: "Wandsworth" }, { id: "350", label: "Warrington" }, { id: "151", label: "Warwickshire" }, { id: "338", label: "West Berkshire" }, { id: "339", label: "West Dunbartonshire" }, { id: "344", label: "West Lothian" }, { id: "152", label: "West Midlands" }, { id: "157", label: "West Sussex" }, { id: "365", label: "West Yorkshire" }, { id: "153", label: "Western Cape" }, { id: "352", label: "Westminster" }, { id: "341", label: "Wigan" }, { id: "154", label: "Wiltshire" }, { id: "347", label: "Windsor and Maidenhead" }, { id: "349", label: "Wirral" }, { id: "348", label: "Wokingham" }, { id: "345", label: "Wolverhampton" }, { id: "155", label: "Worcestershire" }, { id: "351", label: "Wrexham" }, { id: "156", label: "York" }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do these state fields do here?
'DevITJobs' => DevitFormFiller | ||
'BambooHR' => BambooFormFiller, | ||
'DevITJobs' => DevitFormFiller, | ||
'Workable' => WorkableFormFiller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to work out how best to route this in due course
app/services/applier/form_filler.rb
Outdated
@@ -92,10 +94,15 @@ def handle_boolean = (boolean_field.click if @value) | |||
|
|||
def handle_checkbox = check(@value) | |||
|
|||
def handle_date_picker | |||
puts "date_picker is not a valid type!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on here...?
def default_attribute(key) | ||
key.underscore.first(60).gsub(' ', '_').gsub('.', '_') | ||
end | ||
def attributes_dictionary = ATTRIBUTES_DICTIONARY | ||
|
||
ATTRIBUTES_DICTIONARY = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we discuss the difference between the ATTRIBUTES_DICTIONARY & INPUT_TYPES. I would like to understand how this differs across ATS systems
@@ -2,6 +2,7 @@ | |||
<code class="language-json"> | |||
<span><span style="color: var(--shiki-color-text)">{</span></span> | |||
<% questions.each do |question| %> | |||
<% next unless question.payload(job_application)[:interaction] %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do?
@@ -18,10 +18,13 @@ | |||
cron: '0 1 * * *' # Runs once per day (at 1 am) | |||
class: Updater::ExistingCompanyJobsJob | |||
queue: updates | |||
devitjobs_updater: | |||
enabled: <%= Rails.env.production? %> # production only as it interferes with end-to-end testing | |||
devitjobs_updater: # off temporarily as interferes with end-to-end testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this interferes and revert this in due course
|
||
namespace :tests do | ||
# Call this with e.g. rake "tests:end_to_end[https://apply.workable.com/starling-bank/j/60ACE7278C, 15]" | ||
desc "Conduct end-to-end tests on a given url" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some notes on how it works
def complete_all_fields | ||
within application_form do | ||
attach_resume | ||
complete_input_fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely laid out
No description provided.