Issue365 add jobs

[leenyburger]

Description of changes

Added job functionality. Includes the following (copied from issue):
new db table new GET index endpoint new resource in ActiveAdmin test coverage API docs

Jobs table to include: title source_url source city state country description status (i.e. active, inactive) remote Also include tagging.

Issue Resolved

Fixes #365

[jjhampton]

@leenyburger @hpjaj Any reason that status has to be a string? When using this data in the front-end, it seems a bit fragile to match on strings, especially if they're user-entered via Admin Dashboard, where there's a chance for human typos. Thoughts on using boolean instead? Example: active: true or active: false.

[hpjaj]

@jjhampton - We would create a few constants to ensure data integrity. And a dropdown in ActiveAdmin to choose from.

That said, we can definitely switch to a boolean if you don't foresee any use case where more than two statuses would ever be of value?

If we don't see any value there, then I agree, changing the column name to active and type to boolean would be a good call.

[jjhampton]

@hpjaj I've already coded up the front-end stuff to filter on for status === "active" for now, should be fine for the time being. POC here is @ksmacleod99 - Please see above comments - do you envision ever needing to set a Job's status to anything other than just active / inactive?

[hpjaj]

re: @jjhampton 's comment here: https://github.com/OperationCode/operationcode_backend/pull/366#issuecomment-394116169

I agree, would be easier to go the boolean route. If that is still on the table (as I know it would require a bit of a refactor on the FE), I would recommend that, as it is for sure the cleanest solution.

If not, then @leenyburger you'll want to follow this pattern in order to implement choices for "active" and "inactive":

https://github.com/OperationCode/operationcode_backend/pull/375#discussion_r202518607

After this piece is wrapped up, this PR should be ready to ship.

[dmarchante]

Can you review the conflicts and address the final comments from @hpjaj @leenyburger?

[leenyburger]

@dmarchante Yes. I'm out of town until next week but can take a look then.

[apex-omontgomery]

Hi @leenyburger how is this going? Is there anything blocking you that I can assist with?

[leenyburger]

@wimo7083 No blockers, just haven't been able to find the time. Making up for hurricane delay next week, can take a look the week of the 24th.

[leenyburger]

@wimo7083 I'm not going to be able to get to this in a timely manner. Sorry to leave you hanging!

[apex-omontgomery]

Okay so I'm going to make the decision we want to use a boolean instead of a string. I think this would align better with what the frontend needs.

For this I'd recommend:

1. changing status to is_active_status
2. changing all the mocks to respect a boolean instead of string
3. changing the schema to respect the new type.

If nobody objects I'd like to finish this up.

[robbkidd]

I have a maybe philosophical question: what does it mean for a job to have an "active status"?

[kylemh]

@robbkidd Agreed. I feel like that field is unnecessary. I'd prefer a self-cleaning database.

We could end up having tons of inactive jobs

[apex-omontgomery]

Since we don't know when the job will expire. it's probably best to make it adjustable. I can add something to the route so we eliminate innactive jobs.

I went with is_open because apparently using open in Factory girl gave me a security warning.

[robbkidd]

🤔 One option for flagging jobs as either open or closed is to record the date on which it closed. This is a bit like a technique named "soft delete." It would involve adding a datetime column closed_at (if you wanted to match the Rails timestamps style). A nil closed_at means the job is still open; a non-nil closed_at means the job is closed and you also know the date it closed. Knowing the closed date means old-and-closed jobs could be removed from the system.

class Job

  scope :open, -> { where(closed_at: nil) }
  scope :closed, -> { where.not(closed_at: nil) }

  def open?
    self.closed_at.nil?
  end

  def closed?
    !open?
  end
end

There is an open method on Kernel that does IO object instantiation. That might have been the cause of the error from FactoriGirl. It is safe to override that method here with the query scope. The Job class is probably not going to use that Kernel method directly to open a file or some other IO stream.