Skip to content

Commit

Permalink
Merge adf9caa into 0b40b14
Browse files Browse the repository at this point in the history
  • Loading branch information
Neilerino committed Oct 18, 2019
2 parents 0b40b14 + adf9caa commit ceacb3f
Show file tree
Hide file tree
Showing 2 changed files with 242 additions and 0 deletions.
129 changes: 129 additions & 0 deletions adr/ADR-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# [Title of the ADR ]

<!-- There are two main reason why an ADR is needed for a feature
If the feature lasts more then one sprint
Or if a non-obvious design choice was chosen during feature development. -->

## Admin

- **Author:** [ Name of the person you developed the feature ]
- **Deciders:** [ Anybody who is reviewing your code and this ADR ]
- **Date:** [ The date of the pull request ]
- **JIRA:** [ The JIRA number associated with feature ]
- **User Story:** [ A link to the JIRA ]

---

## Context and Problem Statement

[ Quick 2-3 sentence background of the User Story. It should describe the context of the changes being made by the feature, and the problem that the feature is solving. ]

---

## Decision Drivers

### Context

[ The main decision drivers were to be able to add the ViewedByCustomer component to each result, without adding it a second time, when the option in the UserActions component is true. ]

### Decisions

[ A list of the key decisions which had to made during development ]

1. [ Describe decision 1 ]
1. [ Describe decision 2 ]
1. [ Describe decision 3 ]\
...
N. [ Describe decision N ]

---

## Considered Options

<!-- Give some options regarding the decision drivers mentioned in the previous section -->

**Decision 1** - [ Quick option overview decision 1 from the previous section ]

- [Option 1] - [ Describe option 1 ]
- [Option 2] - [ Describe option 2 ]
- ...
- [Option N] - [ Describe option N ]

**Decision 2** - [ Quick option overview decision 2 from the previous section ]

- [Option 1] - [ Describe option 1 ]
- [Option 2] - [ Describe option 2 ]
- ...
- [Option N] - [ Describe option N ]

**Decision 3** - [ Quick option overview decision 3 from the previous section ]

- [Option 1] - [ Describe option 1 ]
- [Option 2] - [ Describe option 2 ]
- ...
- [Option N] - [ Describe option N ]

...

**Decision N** - [ Quick option overview decision N from the previous section ]

- [Option 1] - [ Describe option 1 ]
- [Option 2] - [ Describe option 2 ]
- ...
- [Option N] - [ Describe option N ]

---

## Decision Outcome

#### Decision 1: [Option X]

[ Give your reasoning behind choosing the option you did from the previous section ]

#### Decision 2: [Option X]

[ Give your reasoning behind choosing the option you did from the previous section ]

#### Decision 3: [Option X]

[ Give your reasoning behind choosing the option you did from the previous section ]

...

#### Decision N: [Option X]

From what I understand it makes sense for this feature to be true by default, as it's important for their not to be inconsistencies

---

## Pros & Cons

<!-- List the pros and cons of the options that you chose when developing the feature -->

### Pros

[ List the advantages of developing the feature the way you did ]

### Cons

[ List any cons that come with developing the feature the way that you did ]

---

## Common Pitfalls

[ List any common mistakes that you saw during development of your feature ]

---

## Next Steps & Timeline

[ List any tasks that may follow this one, and the timeline associated with those tasks and the overall feature ]

---

## Related Links

[ List any links related to this feature ie. pull requests, confluence documents, etc.. ]

---
113 changes: 113 additions & 0 deletions adr/components/UserActions/UserActions/ShowViewedByCustomer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# showViewedByCustomer Method (SFINT-2519)

## Admin

- **Author:** Neil Wadden
- **Deciders:** Louis Bompart, Nathan Lafrance-Berger, Andre Theriault, Jeremie Robert
- **Date:** 10/10/2019
- **JIRA:** SFINT-2519
- **User Story:** https://coveord.atlassian.net/browse/SFINT-2519

---

## Context and Problem Statement

<!-- Quick 2-3 sentence background of the User Story -->

The `ViewedByCustomer` component needs to be added on each result template, and therefore could be accidentally missed on one or more, especially if a new template is added at a later time. This would create an inconsistent view of what content the customer has viewed. - From JIRA

---

## Decision Drivers <!-- optional -->

### Context

The main decision drivers were to be able to add the ViewedByCustomer component to each result, without adding it a second time, when the option in the UserActions component is true.

<!-- Number these so that they are easier to reference in the following section -->

### Decisions

1. Need to choose when to edit the results (i.e. need an event)
1. Ensure the `ViewedByCustomer` component is properly added to each result template
1. Ensure that if a template already has the `ViewedByCustomer` component that it won't add a second component
1. There should be an option whether or not to perform that aforementioned actions with the component

---

## Considered Options

<!-- Give some options regarding the decision drivers mentions in the previous section -->

**Decision 1** - What event should be used

- [Option 1] - Leverage the `newResultsDisplayed` event, and loop over every result, performing further action.
- [Option 2] - Leverage the `newResultDisplayed` event, and perform further action.

**Decision 2** - Properly adding the ViewedByDocument Component

- [Option 1] - Add the component using `<div class="CoveoViewedByCustomer">`.
- [Option 2] - Add the component using the `ViewedByCustomer` constructor.

**Decision 3** - Ensure we don't add the template a second time

- [Option 1] - Query the results `HTMLElement` using the `getElementsByClassName` method.
- [Option 2] - Query the results `HTMLElement` using the `querySelectorAll` method.

**Decision 4** - There should be an option whether or not to add the component

- [Option 1] - Have the option be false by default.
- [Option 2] - Have the option be true by default.

---

## Decision Outcome

#### Decision 1: [Option 2]

There are two reason behind this decision selection: First the `newResultsDisplayed` option wasn't passing back the `args.item`, which would have made editing the dom element harder. Second, using the event trigger instead of a for loop made the methods functionality more simple.

#### Decision 2: [Option 2]

The `newResultDisplayed` dom element was firing after the completion of the search-ui, therefore using the `<div>` wasn't possible.

#### Decision 3: [Option 1]

Choosing to use `getElementsByClassName`, in this context I don't think there is a difference between using `querySelectorAll` and `getElementsByClassName`

#### Decision 4: [Option 2]

From what I understand it makes sense for this feature to be true by default, as it's important for their not to be inconsistencies

---

## Pros & Cons

### Pros

- Using the `newResultsDisplayed` method offers an effective way to make dynamic changes to the reuslts, and the functionality of this function could easily be expanded on

### Cons

- Having to call the constructor of `ViewedByCustomer` is a little sketchy. From what I can tell it's not being done anywhere else in the code. It means that a change in the ViewedByCustomer component could negatively affect this method

---

## Gotchas

- The main gotcha for this going forward was that the `newResultsDisplayed` option wasn't passing back the `args.item` with it's event.

---

## Next Steps & Timeline

This was a standalone task and has no additional steps past this.

---

## Related Links

- **search-ui-extensions PR**: https://github.com/coveo/search-ui-extensions/pull/45
- **salesforceIntegrationV2 PR**: https://bitbucket.org/coveord/salesforceintegrationv2/pull-requests/1470/

---

0 comments on commit ceacb3f

Please sign in to comment.