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

Documentation says Field::URL accepts option for truncate but code is different #2597

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thebravoman
Copy link

Documentation at https://administrate-demo.herokuapp.com/customizing_dashboards
Code was not calling truncate.
With current change truncate is called.

This is a clean PR along with a spec. Original PR is at
#2544
but it was mostly to get some feedback.

@pablobm
Copy link
Collaborator

pablobm commented Jul 19, 2024

Thank you for this, and apologies for the delay to look at it. One problem is that the spec doesn't test the functionality as it's all stubbed. I understand it still needed updating to avoid it breaking.

Field::String and Field::Text are also affected. Would you be up for fixing those and see if you can provide specs that do test for regressions?

@thebravoman
Copy link
Author

I was thinking about this and tried staying with the current way tests are written and have a minimal change. Calling the method with less stubbing would mean moving a little further than what is currently in the specs. It will require probably a new type of spec. I could think about it.

@pablobm
Copy link
Collaborator

pablobm commented Jul 26, 2024

@thebravoman - If you could give it a go, that would be great 🙂 If after some trying, you find it's too complex or otherwise have difficulty, let us know here and we can rethink it.

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