Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new report - member registrations #2492

Open
wants to merge 11 commits into
base: 2.9
Choose a base branch
from

Conversation

error08
Copy link
Contributor

@error08 error08 commented Oct 14, 2023

Add new report member registrations using new apexcharts library.

@aschempp
Copy link
Member

Unfortunately, I get an error message when trying this locally:

An exception occurred while executing a query: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '(ORDER BY m.dateAdded) as all_members,
DATE_FORMAT(FROM_UNIXTIME' at line 3

Since I'm running MySQL 5.7 (MAMP on macOS), it could be that ORDER BY inside the SELECT is not supported on older databases?

@error08
Copy link
Contributor Author

error08 commented Oct 17, 2023

Yes you are right! The window functions (sum in that case) are new in Version 8. But 5.7 has reached EOL. Should we support that version futhermore? How do you handle the base requirements for Isotope? Can they only be updated with a new major version of Isotope?

@aschempp
Copy link
Member

Well we cannot require a minimum version of MySQL (technically, on a Composer level). And Contao perfectly supports MySQL 5.7. So it would be a PITA to have a different dependency. Appart from the fact that my default hoster still has 5.7 (will be changed early next year) 😅

@error08
Copy link
Contributor Author

error08 commented Oct 23, 2023

Ok, I see! I will try an alternative query.

@error08
Copy link
Contributor Author

error08 commented Nov 13, 2023

I removed the window function and solved it via code. I think it is a good alternative. Now the code compatible to MySQL 5.x.

@aschempp aschempp added this to the 2.9.0 milestone Dec 6, 2023
@aschempp
Copy link
Member

aschempp commented Dec 6, 2023

I tried this PR locally. But (probably) because I did not have any registration/member data, I got an empty page and an error in apexchart
Unhandled Promise Rejection: TypeError: null is not an object (evaluating 't.toString')

I guess there's a problem somewhere if the data/row is empty?

@error08
Copy link
Contributor Author

error08 commented Dec 8, 2023

Hi, I tried it without members / member data, but I cannot reproduce the error. I need more information about this.

@aschempp
Copy link
Member

aschempp commented Feb 5, 2024

Could you add a screenshot what this report is supposed to look like? Here's what I see. I guess the chart error is because some of the months do not have a new registration?

Bildschirmfoto 2024-02-05 um 10 49 44

@error08
Copy link
Contributor Author

error08 commented Feb 5, 2024

Hi, here a screenshot

image

I have update the PR, please try it again. I see no such error.

image

@error08
Copy link
Contributor Author

error08 commented Feb 23, 2024

@aschempp I think now is fine and ready to merge?

@aschempp
Copy link
Member

I'm sorry, I don't know what I'm doing wrong. I tried it again, but nothing changed. As you can see in the previous screenshot, I don't even have the same columns in the HTML table. Are you sure you pushed all changes?

@error08
Copy link
Contributor Author

error08 commented Mar 6, 2024

I have found the problem. The language keys in the English file was wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants