Delete bills #8
Delete bills #8
Conversation
If an expense cannot be found within a nobt the UnknownExpenseException can be thrown
To remove an expense from a nobt a new Enpoint is provided
| this.expenses.removeIf(e -> e.getId() == expenseId); | ||
|
|
||
| Expense expenseToDelete = expenses.stream() | ||
| .filter(e -> e.getId() == expenseId) |
There was a problem hiding this comment.
Funktioniert der Referenzvergleich hier?
There was a problem hiding this comment.
Ja, ist ein primitive type
| @@ -0,0 +1,3 @@ | |||
| ALTER TABLE expenses | |||
| ADD deleted BOOLEAN; | |||
There was a problem hiding this comment.
Sollten wir das vielleicht NOT NULL machen und alle anderen explizit auf false setzen?
| final Nobt nobtWithoutExpense = fetch(id); | ||
|
|
||
| assertThat(nobtWithoutExpense, hasExpenses( | ||
| iterableWithSize(0) | ||
| )); | ||
| assertThat(nobtWithoutExpense, hasDeletedExpenses( |
There was a problem hiding this comment.
Die beiden Assertions kannst du zusammenfassen auf ein: allOf(hasExpenses(...), hasDeletedExpenses(...))
| @@ -37,6 +37,9 @@ | |||
| @Column(name = "date", nullable = false) | |||
| private LocalDate date; | |||
|
|
|||
| @Column(name = "deleted", nullable = false) | |||
There was a problem hiding this comment.
Wenns hier nullable=false ist, sollte das auch im Migrationskript so in die DB geschrieben werden. Ich würde vielleicht auch insertable=false setzen. Ein hinzufügen einer gelöschten Rechnung macht keinen Sinn.
| @@ -60,6 +72,10 @@ public NobtEntity mapToDatabaseModel(Nobt domainModel) { | |||
| nobtEntity.setExplicitParticipant(domainModel.getParticipatingPersons()); | |||
|
|
|||
| domainModel.getExpenses().stream().map(expenseMapper::mapToDatabaseModel).forEach(nobtEntity::addExpense); | |||
| domainModel.getDeletedExpenses().stream() | |||
| .map(expenseMapper::mapToDatabaseModel) | |||
| .peek(e -> e.setDeleted(true)) | |||
There was a problem hiding this comment.
Da wir eine Expense eigentlich nie mehr auf "nicht deleted" setzen wollen, könnte man in der Expense-Entity eine Methode anbieten die heißt markDeleted() oder so ähnlich und diesen boolean gar nie nach außen preisgibt. Dann kannst du hier auch ne Methodenreferenz verwenden. Kommt halt darauf an, wieviele solche "Convenience"-Methoden man in seiner Entity haben will.
| given(this.documentationSpec) | ||
| .port(config.port()) | ||
| .filter( | ||
| document("delete-expense", |
There was a problem hiding this comment.
Für dieses Snippet müsstest du im index.adoc File noch eine Sektion hinzufügen, wo du es wirklich inkludierst und die eigentliche Doku schreibst.
| final String nobtId = client.createGrillfeierNobt(); | ||
| client.addFleischExpense(nobtId); | ||
|
|
||
| final Long idOfFirstExpense = client.getNobt(nobtId).jsonPath().getLong("expenses[0].id"); |
There was a problem hiding this comment.
Das hier sollte dann den Link aus der Response holen und nicht die ID, sobald der Link mal zurückgeliefert wird.
|
Das hier sollte uns bei der Link-Generierung helfen: https://stackoverflow.com/a/42455398/2489334 Hab mir doch gedacht, dass Jackson hier ein Feature hat. Die Library ist einfach so unglaublich mächtig :D |
The VirtualBeanPropertyWriter allows us to add dynamic properties every time Jackson serializes an instance of our Expense. Through the built-in attribute-passing mechanism of Jackson, we pass the PropertyWriter an instance of our LinkFactory so that the logic for actually creating the link lives outside of the PropertyWriter and can be easily tested. We can now also use the "delete"-link in the ApiDocumentationTest to delete the expense.
| @@ -146,11 +153,26 @@ private void registerTestFailRoute() { | |||
| }); | |||
| } | |||
|
|
|||
| private String serialize(Object entity, Request req, NobtId nobtId) throws JsonProcessingException { | |||
| return objectMapper.writer() | |||
| .withAttribute(ExpenseLinkFactory.class.getName(), new ExpenseLinkFactory(req.scheme(), req.host(), nobtId)) | |||
There was a problem hiding this comment.
Wir müssen Scheme aus dem Header der vom AWS Load-Balancer gesetzt wird (X-Forwarded-Proto), auslesen. (https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html)
Falls nicht vorhanden, sollten wir auf das Scheme zurückfallen, welches Spark hier auslesen kann.
The new config value allows to define an HTTP-header that will is used to override the scheme of generated links, if it contains a value. This is needed for our production environment at AWS because our application sits behind a load-balancer that accepts all requests forwards them to our application. The load-balancer accepts HTTPS requests but forwards them as HTTP. Thus, generated links would be invalid if we use the scheme of the current request to generate the link.
| @@ -136,8 +143,8 @@ private void registerCreateNobtRoute() { | |||
| res.header("Location", req.url() + "/" + nobt.getId().getValue()); | |||
There was a problem hiding this comment.
This link is probably wrong too.
There was a problem hiding this comment.
We should not use url here but create another factory that uses host and scheme.
This prevents DeletedExpenses to render a link for deleting them.
This way, we also know when an expense was deleted.
Fixes #6