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

Fix application_question.locator for payload #227

Open
wants to merge 44 commits into
base: main
Choose a base branch
from

Conversation

patanj101
Copy link
Collaborator

No description provided.

Copy link
Owner

@Ches-ctrl Ches-ctrl left a 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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How so?

Copy link
Owner

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

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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly - why?

Copy link
Collaborator

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.

Copy link
Owner

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

Copy link
Owner

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?
Copy link
Owner

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

Copy link
Collaborator

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work?

Copy link
Owner

@Ches-ctrl Ches-ctrl left a 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
Copy link
Owner

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?

Copy link
Collaborator Author

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')
Copy link
Owner

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?

Copy link
Collaborator Author

@patanj101 patanj101 Aug 9, 2024

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.

Copy link
Owner

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",
Copy link
Owner

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?

Copy link
Collaborator Author

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.

@patanj101 patanj101 temporarily deployed to cheddar-pipe-2024-08-08-iahgw9 August 9, 2024 08:03 Inactive
@patanj101 patanj101 temporarily deployed to cheddar-pipe-2024-08-08-gwq8o8 August 9, 2024 08:08 Inactive
@patanj101 patanj101 temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 9, 2024 08:14 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 9, 2024 11:05 Inactive
@patanj101 patanj101 temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 9, 2024 11:20 Inactive
@patanj101 patanj101 temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 9, 2024 12:34 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 9, 2024 13:05 Inactive
Copy link
Owner

@Ches-ctrl Ches-ctrl left a 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" }]
Copy link
Owner

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
Copy link
Owner

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

@@ -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!"
Copy link
Owner

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 = {
Copy link
Owner

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] %>
Copy link
Owner

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
Copy link
Owner

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"
Copy link
Owner

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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely laid out

@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 12, 2024 14:31 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 12, 2024 14:40 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 12, 2024 16:27 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 13, 2024 08:28 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 13, 2024 10:02 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 13, 2024 13:43 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 13, 2024 17:02 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 13, 2024 17:08 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 14, 2024 07:36 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 15, 2024 08:39 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 16, 2024 12:26 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 16, 2024 17:24 Inactive
@Ches-ctrl Ches-ctrl temporarily deployed to cheddar-pipe-2024-08-08-hl4fwk August 17, 2024 02:38 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants