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

[3.6][WIP] Changelog fixes / touch-ups #7336

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

Conversation

bobdenotter
Copy link
Member

WIP: I'll need to make sure changes in repeaters/blocks are logged.

- Use 'contenttypeslug' to display contenttype correctly
- Remove unused column "Editor's Comment"
- Ignore whitespace for fields with text
- Handle repeaters
</td>
<td class="change-log-item">
{{ field.after.render }}
{# {{ field.after.render }} #}
{{ dump(field.after.render)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Erm … you sure about that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, that's part of the WIP. It's there so it won't break on Repeaters/Blocks. I need to work on that.

$c->get('/changelog/{contenttype}/{contentid}', 'changeLogRecord')
->value('contenttype', '')
$c->get('/changelog/{contenttypeslug}/{contentid}', 'changeLogRecord')
->value('contenttypeslug', '')
Copy link
Contributor

Choose a reason for hiding this comment

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

BC break

Copy link
Member Author

Choose a reason for hiding this comment

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

How come? It's only used in one place, and i'm not actually changing what's passed in, just what's done with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

It changes the router signature

'address' => isset($before['address']) ? : '',
'latitude' => isset($before['latitude']) ? $before['latitude'] : '',
'longitude' => isset($before['longitude']) ? $before['longitude'] : '',
'formatted_address' => isset($before['formatted_address']) ? $before['formatted_address'] : '',
Copy link
Contributor

Choose a reason for hiding this comment

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

All this should be in a Bag

Copy link
Member Author

Choose a reason for hiding this comment

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

Consider it done.

'latitude' => $before['latitude'],
'longitude' => $before['longitude'],
'formatted_address' => $before['formatted_address'],
'address' => isset($before['address']) ? : '',

Choose a reason for hiding this comment

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

shouldnt it be isset($before['address']) ?$before['address'] : '', ?

@bobdenotter bobdenotter changed the base branch from 3.4 to 3.5 March 7, 2018 12:52
@bobdenotter bobdenotter force-pushed the hotfix/changelog-fixes branch 2 times, most recently from 30e1e3e to 10e2c7f Compare March 7, 2018 13:07

$result = [];

foreach($combinedkeys as $key) {

Choose a reason for hiding this comment

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

Expected 1 space after FOREACH keyword; 0 found

@bobdenotter bobdenotter changed the title [WIP] Changelog fixes / touch-ups for Bolt 3.4 Changelog fixes / touch-ups for Bolt 3.5 Mar 7, 2018
@bobdenotter
Copy link
Member Author

It's not perfect, but it's much better than it was. For one, it doesn't crash, and it handles repeaters now too.

screen shot 2018-03-07 at 16 54 25

@bobdenotter
Copy link
Member Author

Oh, and the target is changed from 3.4 to 3.5.

@xiaohutai
Copy link
Member

xiaohutai commented Mar 8, 2018

Somehow it doesn't show the proper before and after changes for blocks and repeaters; I always get the same values left and right:

changelog--blocks-repeaters

Edit: 2018-03-12

It's really weird what's going on. Sometimes it shows both the old values, and somtimes both the new ones.

With repeaters/blocks, you always get to see those items, regardless of whether they have changed or not. Also taxonomy and relation are always showing up.
Try clicking save while not having fields modified. This may be related to abovementioned issue.

Note: It is because there is actually only 1 RepeatingFieldCollection, if it exists, it will start (lazily) fetch the same values from the database.

@bobdenotter bobdenotter changed the title Changelog fixes / touch-ups for Bolt 3.5 [WIP] Changelog fixes / touch-ups for Bolt 3.5 Mar 15, 2018
@bobdenotter bobdenotter changed the title [WIP] Changelog fixes / touch-ups for Bolt 3.5 [3.6][WIP] Changelog fixes / touch-ups for Bolt 3.6 Mar 20, 2018
@stale
Copy link

stale bot commented May 26, 2018

This issue has been automatically marked as stale because it has not had recent activity. Maybe this issue has been fixed in a recent release, or perhaps it is not affecting a lot of people?
It will be closed if no further activity occurs, because we like to keep the issue queue concise and actual.
If you think this issue is still relevant, please let us know. Especially if you’d like to help resolve the issue, either by helping us pinpointing the cause of a bug, or in implementing a fix or new feature.

@stale stale bot added stale Stale issues & PRs flagged for closing and removed stale Stale issues & PRs flagged for closing labels May 26, 2018
@xiaohutai xiaohutai added the keep label May 28, 2018
@xiaohutai
Copy link
Member

Still needs more investigation [and still a useful feature to have].

Could be fixed by serializing the before record/values (iterate over all values in lazy items)?

@bobdenotter
Copy link
Member Author

Yes, should absolutely keep this.. I've put it on hold, to fix up after #7291 has landed, because then we can fix it properly :-)

@GwendolenLynch GwendolenLynch changed the title [3.6][WIP] Changelog fixes / touch-ups for Bolt 3.6 [3.6][WIP] Changelog fixes / touch-ups Jun 10, 2018
@GwendolenLynch GwendolenLynch changed the base branch from 3.5 to 3.6 June 10, 2018 14:42
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

5 participants