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

Some small fixes #24

Merged
merged 4 commits into from
Feb 20, 2024
Merged

Some small fixes #24

merged 4 commits into from
Feb 20, 2024

Conversation

rimas-kudelis
Copy link
Contributor

@rimas-kudelis rimas-kudelis commented Jan 14, 2024

I think these are small enough to be in a single PR, at least in this case. The changes proposed here are:

  • using LEFT JOIN in SELECT queries instead of WHERE. This allows exporting articles with missing authors and/or categories
  • JoomlaContent::isPublished() now only returns true if the article was marked as Published in Joomla. Previously it would return true for Unpublished articles as well, but not for Archived or Trashed articles, and was used to filter out which articles not to export at all. After this update, all articles will be exported, but those not currently Published in Joomla will be marked as draft: true. Perhaps the filtering logic could be reintroduced, but use some new property.
  • the created date of Joomla articles is now exported as date, and modified date as lastmod (previously, only modified date was exported as date). I myself actually used the publish_up value as date field, but that should probably be exported into the publishDate field instead. I didn't try that scenario though, so leaving this nuance as-is.
  • two seemingly unused *.toml.ftl templates have been removed.

I didn't even look at tests, so it's possible that some are now failing.

@rimas-kudelis
Copy link
Contributor Author

@davetcc, care to review? I'd like to delete my fork already.

@davetcc
Copy link
Owner

davetcc commented Feb 20, 2024

Apologies, I had not noticed this, I will review immediately.

Copy link
Owner

@davetcc davetcc left a comment

Choose a reason for hiding this comment

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

All good, thanks for raising the PR, and apologies for the delay in reviewing.

@davetcc davetcc closed this Feb 20, 2024
@davetcc davetcc reopened this Feb 20, 2024
@davetcc davetcc merged commit 8b89562 into davetcc:master Feb 20, 2024
@rimas-kudelis
Copy link
Contributor Author

Thank you! :)

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

Successfully merging this pull request may close these issues.

2 participants