Skip to content

Maintenance - #19

Merged
JanHolger merged 56 commits into
masterfrom
maintenance
May 28, 2021
Merged

Maintenance#19
JanHolger merged 56 commits into
masterfrom
maintenance

Conversation

@JanHolger

@JanHolgerJanHolger commented Mar 30, 2021

Copy link
Copy Markdown
Member

This pull request is supposed to redesign the query building process to allow implenting other dialects aswell as fixing issues that came up in the last months.

TimothyGillespie
TimothyGillespie reviewed Mar 31, 2021
View reviewed changes

@TimothyGillespieTimothyGillespie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As it is only a draft I only gave it a cursory look. When it is ready for review, please mark request me for review, since I'm not sure if I will beinformed about any status change.

Comment threadsrc/main/java/org/javawebstack/orm/query/QueryCondition.java
Comment threadsrc/main/java/org/javawebstack/orm/query/QueryCondition.java
Comment threadsrc/main/java/org/javawebstack/orm/query/Query.java
TimothyGillespieand others added 14 commits March 31, 2021 04:41
@TimothyGillespie
Merge branch 'master' into TimothyGillespie/addOrderByTests
480b883
@TimothyGillespie
Add assertOrderByContains method
49e4fa0
@TimothyGillespie
Add SectionIndexOutOfBoundException
8aaedb2
@TimothyGillespie
Add retrieval methods for the sections
1ae8ac8
@TimothyGillespie
Add JavaDocs for getSectionList too
5d4e233
@TimothyGillespie
Add JavaDocs for getSectionList too
89c48dc
@TimothyGillespie
Reorder methods more logically
51907a4
@TimothyGillespie
Add assertSectionEquals method
ac7140e
@TimothyGillespie
Remove OrderBy specific contains method
5491a48
@TimothyGillespie
Make assertion methods chainable
6f97433
@TimothyGillespie
Add better fitting internal assert for the equal assertion
b73d7db
@TimothyGillespie
Add tests for OrderBy Clauses
710b6c5
@TimothyGillespie
Merge branch 'TimothyGillespie/addOrderByTests' of github.com:JavaWeb…
9e857cf
…Stack/ORM into TimothyGillespie/addOrderByTests
@JanHolger
Added extra fields to Model and added QueryWith but didn't yet implem…
0201c3d
…ent it in the query builder as i'm way too tired (:
TimothyGillespie
TimothyGillespie reviewed Apr 1, 2021
View reviewed changes
Comment threadsrc/main/java/org/javawebstack/orm/query/QueryColumn.java

private static void validateName(String name) {
if(!NAME_PATTERN.matcher(name).matches())
throw new ORMQueryException("Invalid column name '" + name + "' (Use raw in case you know what you're doing)");

@TimothyGillespieTimothyGillespieApr 1, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This solves the other aspect of the issue of #14. If this branch is merged the issue can be closed.

Comment threadsrc/main/java/org/javawebstack/orm/query/QueryCondition.java
Comment threadsrc/main/java/org/javawebstack/orm/query/QueryCondition.java
@TimothyGillespieTimothyGillespie linked an issue Apr 1, 2021 that may be closed by this pull request
Potential Issue with where clauses #14
Closed
@TimothyGillespieTimothyGillespie linked an issue Apr 2, 2021 that may be closed by this pull request
Only one order by can be handled #22
Closed
@TimothyGillespieTimothyGillespie mentioned this pull request Apr 2, 2021
Closed
@JanHolger
JanHolger marked this pull request as ready for review April 27, 2021 10:49
@TimothyGillespie

TimothyGillespie commented Apr 27, 2021

Copy link
Copy Markdown
Contributor

I will review this tomorrow or later this evening.

TimothyGillespie
TimothyGillespie reviewed Apr 28, 2021
View reviewed changes

@TimothyGillespieTimothyGillespie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See no issue with this merge request when #26 is merged or otherwise addressed.

@TimothyGillespie

TimothyGillespie commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

I changed #26 as discussed

@JanHolger
JanHolger merged commit 3d77e1c into masterMay 28, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@TimothyGillespieTimothyGillespieTimothyGillespie left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Only one order by can be handled Potential Issue with where clauses

2 participants

@JanHolger@TimothyGillespie