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

Added support for date modified field on cards #3346

Merged
merged 40 commits into from
Feb 4, 2021

Conversation

grafitto
Copy link
Contributor

fixes #1893

PR checklist:

  • Update READ.me ?
  • Update API documentation ?

QA checklist:

  • Smoke test the functionality described in the issue
  • Test for side effects
  • UI responsiveness
  • Cross browser testing
  • Code review

@grafitto grafitto marked this pull request as draft November 27, 2020 19:59
@grafitto grafitto marked this pull request as ready for review December 9, 2020 12:16
@RafaPolit RafaPolit requested a review from daneryl December 22, 2020 14:08
@daneryl daneryl self-assigned this Dec 22, 2020
Copy link
Collaborator

@daneryl daneryl left a comment

Choose a reason for hiding this comment

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

some tests are failing, also some tests are missing, the details are in the code comments.

app/api/entities/entities.js Show resolved Hide resolved
app/react/Library/components/specs/SortButtons.spec.js Outdated Show resolved Hide resolved
app/react/Metadata/helpers/formater.js Outdated Show resolved Hide resolved
app/react/Metadata/helpers/formater.js Show resolved Hide resolved
app/react/Metadata/helpers/formater.js Show resolved Hide resolved
app/server.js Outdated Show resolved Hide resolved
@grafitto grafitto marked this pull request as draft January 12, 2021 14:18
Copy link
Collaborator

@daneryl daneryl left a comment

Choose a reason for hiding this comment

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

does not work properly on already existing collections, to replicate.

  • yarn blank state
  • modify an entity
  • sorting by date modified (recently or least recently) results on the modified entity showing on the last position.

maybe this needs a migration to add the date modified property ? @grafitto @RafaPolit ?

@grafitto
Copy link
Contributor Author

does not work properly on already existing collections, to replicate.

  • yarn blank state
  • modify an entity
  • sorting by date modified (recently or least recently) results on the modified entity showing on the last position.

maybe this needs a migration to add the date modified property ? @grafitto @RafaPolit ?

I think it needs migration...

@daneryl
Copy link
Collaborator

daneryl commented Jan 18, 2021

  • yarn blank state
  • modify an entity
  • sorting by date modified (recently or least recently) results on the modified entity showing on the last position.

this is wrong, the proper way to replicate is to use existing data, (yarn fixtures, instead of blank-state)

@grafitto
Copy link
Contributor Author

  • yarn blank state
  • modify an entity
  • sorting by date modified (recently or least recently) results on the modified entity showing on the last position.

this is wrong, the proper way to replicate is to use existing data, (yarn fixtures, instead of blank-state)

Noted. After testing, I think the editDate property is not added when an entity is edited

@grafitto
Copy link
Contributor Author

Only the entities with the editDate property are being sorted, the rest are not sorted and remain the same. If you edit more than one entities, you can actually see them getting sorted but the rest stay in the same position. @daneryl

@grafitto grafitto requested a review from daneryl February 1, 2021 12:18
Copy link
Collaborator

@daneryl daneryl left a comment

Choose a reason for hiding this comment

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

@grafitto migrations looks good.
i have noticed an issue where sorting by the new "date modified" results in the same order whether it is sorted by "recently" or "least recently"

@grafitto
Copy link
Contributor Author

grafitto commented Feb 2, 2021

@grafitto migrations looks good.
i have noticed an issue where sorting by the new "date modified" results in the same order whether it is sorted by "recently" or "least recently"

Does this happen even after you run the migration?

@daneryl
Copy link
Collaborator

daneryl commented Feb 2, 2021

@grafitto migrations looks good.
i have noticed an issue where sorting by the new "date modified" results in the same order whether it is sorted by "recently" or "least recently"

Does this happen even after you run the migration?

yes, does it work properly for you ?

@grafitto
Copy link
Contributor Author

grafitto commented Feb 2, 2021

@grafitto migrations looks good.
i have noticed an issue where sorting by the new "date modified" results in the same order whether it is sorted by "recently" or "least recently"

Does this happen even after you run the migration?

yes, does it work properly for you ?

Mine seems to work as expected. I run yarn migrate and yarn reindex, I am not sure if reindexing helps. You could try that

@daneryl
Copy link
Collaborator

daneryl commented Feb 2, 2021

@grafitto migrations looks good.
i have noticed an issue where sorting by the new "date modified" results in the same order whether it is sorted by "recently" or "least recently"

Does this happen even after you run the migration?

yes, does it work properly for you ?

Mine seems to work as expected. I run yarn migrate and yarn reindex, I am not sure if reindexing helps. You could try that

@grafitto this was an issue on my side, sorting is working properly now, im not sure what happened may be i forgot a reindex.

sorting apparently works properly but the date shown on the card for "Date modified" its always today not the actual date modified, it can be that the new field is somehow excluded from the results, please review.

@grafitto
Copy link
Contributor Author

grafitto commented Feb 3, 2021

@grafitto migrations looks good.
i have noticed an issue where sorting by the new "date modified" results in the same order whether it is sorted by "recently" or "least recently"

Does this happen even after you run the migration?

yes, does it work properly for you ?

Mine seems to work as expected. I run yarn migrate and yarn reindex, I am not sure if reindexing helps. You could try that

@grafitto this was an issue on my side, sorting is working properly now, im not sure what happened may be i forgot a reindex.

sorting apparently works properly but the date shown on the card for "Date modified" its always today not the actual date modified, it can be that the new field is somehow excluded from the results, please review.

On it

@grafitto
Copy link
Contributor Author

grafitto commented Feb 3, 2021

@daneryl I have added editDate to documentQueryBuilder, kindly check whether that fixes the issue

@daneryl daneryl merged commit 133dd39 into development Feb 4, 2021
@daneryl daneryl deleted the 1893-add-date-modified branch February 4, 2021 13:26
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.

Request/Idea: Add an automated "Date updated" property option
3 participants