[CLOSED] Don't index preview_only Finders on Production

[benilovj]

Issue by tommyp Monday 21 September 2015 at 14:27 GMT Originally opened as https://github.com/alphagov/specialist-publisher/pull/571

---

In order to prevent Finders going into Elastic Search on Production we use the preview_only flag to decide if it should be published or not. This commit follows the same logic in the PublishingApiFinderPublisher but keeps them separate. There's nothing to stop these being merged in future but as this is preventing deploys I thought I'd go down the simplest route now.

This PR also removes the funtionality which stops Finders being deployed without a content ID. As we can now restrict them to Preview, there's no need to keep this feature around.

Ticket.

---

tommyp included the following code: https://github.com/alphagov/specialist-publisher/pull/571/commits

[benilovj]

Comment by rboulton Monday 21 September 2015 at 14:57 GMT

---

I think the title of this PR may be reversed: it looks like the intention is to index preview_only? finders on preview. Should the title be "Don't index preview_only Finders on Production"?

[benilovj]

Comment by tommyp Monday 21 September 2015 at 14:58 GMT

---

@rboulton Yes. You're right. Changed the title.

[benilovj]

Comment by rboulton Monday 21 September 2015 at 15:03 GMT

---

Nitpick (but important) - the commit message also has the same problem as the title had.

Publishing these finders only in preview seems like a reasonable approach to the problem. I'm not sure if there's a better way to check whether the code is running in a preview environment. Perhaps having our deployment process set up a configuration value saying whether "preview_only?" finders should be published would be a better approach?

[benilovj]

Comment by jennyd Monday 21 September 2015 at 15:04 GMT

---

We've got a separate story to switch to using a new specific environment variable for this feature flag, rather than relying on GOVUK_APP_DOMAIN.

[benilovj]

Comment by tommyp Wednesday 23 September 2015 at 14:30 GMT

---

@rboulton @jennyd I've changed the commit message as well and updated the commit to remove the ability to restrict Finders from deploy which don't have a content_id.

[benilovj]

Comment by jennyd Wednesday 23 September 2015 at 14:38 GMT

---

Could you separate this into 2 commits, one for removing the logic around content_id and one for the RummagerFinderPublisher changes?

[benilovj]

Comment by tommyp Wednesday 23 September 2015 at 14:43 GMT

---

@jennyd Done 😀

[benilovj]

Comment by mikejustdoit Thursday 24 September 2015 at 08:44 GMT

---

Now that we're not using empty content_ids, do we need to validate that a Finder's content_id isn't empty elsewhere? Like, in the schemas and/or validators.

And does this have any impact on the other apps that interact with finders?

[benilovj]

Comment by rboulton Thursday 24 September 2015 at 09:28 GMT

---

It can't quite be made mandatory in the schemas yet, because some apps are still sending things without content ids. My team is currently working on fixing a lot of those apps, but I'm not sure we'll get them all done. Once we're in a position to satisfy it though, we do want to make content_ids mandatory in the schemas, at which point hopefully schema tests will help ensure that they're not empty.

[benilovj]

Comment by tommyp Thursday 24 September 2015 at 09:32 GMT

---

@mikejustdoit The content_id is called using fetch, so it will raise an exception if it isn't present. This is the same behaviour that we have for the other fields that the Publishing API uses at the root level of the hash.

[benilovj]

Comment by mikejustdoit Thursday 24 September 2015 at 10:06 GMT

---

All right, I'm happy with this, then :+1: